Erase operation on vector not working

Question

I am new to C++ and I am having difficulties with getting the vector.erase operation to work.

I have a database class as such:

template <class T> class Database
{
    protected:
        std::vector<T> m_database;
        int m_counter;
    public:
        Database();
        virtual ~Database();

        // Accessor Methods.
        T& getObject(int objectId);
        bool exists(int objectId);

        // Mutator Methods.
        void add(T Object);
        void del(int objectId);
};

and in practice, I am using the code as such:

Database<Account> accountDatabase;

Account is a base class, with two derived classes, ChequingAccount and SavingsAccount.

I am inserting accounts, regardless of type (could be Account, ChequingAccount, SavingsAccount) into this database using:

template <class T> void Database<T>::add(T object)
{
    m_database.push_back(object);
    ++m_counter;
}

However, I am having issues with my delete operation. I am searching for a corresponding objectId and then deleting it from the vector.

// Deletes the specified object from the database.
template <class T> void Database<T>::del(int objectId)
{
    std::vector<T>& database             = m_database;
    typename std::vector<T>::iterator it = database.begin();

    while (it != database.end()) {
        if ((*it).getId() == objectId) {
            it = database.erase(it);
        } else {
            ++it;
        }
    }
}

Unfortunately the delete operation is not working at all. I am also having issues pulling a derived class from the database, as everything is being pulled out as an Account type. I believe these two issues are tied to my noob C++ skills and bad design.

Any help would be greatly appreciated! Thanks!

EDIT

By not working, I mean the object is not deleted from the database. I apologize for any confusion.

The Account classes:

class Account
{
    protected:
        int m_id;
        double m_balance;
        std::string m_name, m_type;
    public:
        Account(int id, int userId, double balance = 0, std::string name = ""); // Constructor.
        ~Account(); // Destructor.

        // Accessor Methods.
        // This returns m_id AKA objectId
        int getId() const;
}

class ChequingAccount: public Account
{
    public:
        ChequingAccount(int id, int userId, double balance, std::string name) : Account(id, userId, balance, name) {}
}

class SavingsAccount: public Account
{
    public:
        SavingsAccount(int id, int userId, double balance, std::string name) : Account(id, userId, balance, name) {}
}

1. What do you mean by "not working at all"? 2. Why not use `std::remove`? — Billy ONeal, Sep 23 '14 at 19:34
possible duplicate of [deleting while iterating](http://stackoverflow.com/questions/3901356/deleting-while-iterating) — Raymond Chen, Sep 23 '14 at 19:35
@Raymond This is slightly different because the user is correctly reassigning the iterator result from the call to `std::vector::erase(iterator)` — Billy ONeal, Sep 23 '14 at 19:38
Your `del()` looks fine so maybe check if your `id` is correct? — Galik, Sep 23 '14 at 19:40
Also you can't store derived class objects in there unless you are storing pointers rather than the objects themselves. — Galik, Sep 23 '14 at 19:43
My apologies for the confusion, I clarified a bit more. Thanks for the insight Galik, I would like to store the derived classes, seems like I need to use pointers. — user3745117, Sep 23 '14 at 19:46
I have included my `Account` classes to the question if it helps — user3745117, Sep 23 '14 at 19:55
How does one get an object ID? You have methods that have ID parameters, but none that return one. — Thomas Matthews, Sep 23 '14 at 19:57

score 2 · Answer 1 · answered Sep 23 '14 at 19:36

2

Note that your remove function would be far better in terms of std::remove_if:

auto endIterator = std::remove_if(m_database.begin(), m_database.end(),
    [=](T const& entry) { return entry.getId() == objectId; }
    );
m_database.erase(endIterator, m_database.end());

That said, your version does not look incorrect, just inefficient. What about it "doesn't work"? I do note that you maintain a separate object ID in your database but never actually put that ID into the objects you store.

answered Sep 23 '14 at 19:36

Billy ONeal

104,103
58
317
552

the old 'erase/remove' idiom – Marco A. Sep 23 '14 at 19:37
Assuming that my method is "correct", it should be my objectId giving me problems? – user3745117 Sep 23 '14 at 19:47
@user3745117 you never show how the ID gets into T so we have no idea. – Billy ONeal Sep 23 '14 at 19:49
@BillyONeal I have edited my question to include those classes. – user3745117 Sep 23 '14 at 19:54

score 1 · Answer 2 · edited May 23 '17 at 12:06

1

Your del function looks correct, only suggestion is instead of:

(*it).getId()

you can/should use:

it->getId()

which is simpler and more readable. So culprit is somewhere else. As for issue, that everything is pulled as Account type, you have slicing problem, details read here What is object slicing?

The fact that you make your class Database a template does not change anything, you probably misunderstand what template does. Your Database<Account> is nothing more than as you would use Account instead of T and not use template at all, and you insert objects, inherited from Account by value.

edited May 23 '17 at 12:06

Community

1
1

answered Sep 23 '14 at 19:50

Slava

43,454
1
47
90

Yes, I suspected this was the case for losing the derived type. I could not get a vector of pointers working properly, however. I guess I shall try again – user3745117 Sep 23 '14 at 19:56

Erase operation on vector not working

2 Answers2