C++ Erasing element from a vector of pointers

102 views Asked by At

I am using a vector of pointers for inheritance purposes, and I have every behaving as it should, however, I am having issues deleting objects from the vector.

I have created a Github gist with all the source files, along with a Makefile for easy compiling and running:

https://gist.github.com/anonymous/7c689940992f5986f51e

Essentially here is the problem:

I have a Bank class as such (note the Account Database structure, which is an encapsulated vector of Account*):

class Bank
{
private:
    Database<Account> m_acctDb;
    Database<User> m_userDb;
    int m_accountCounter;
    int m_userCounter;
public:
    Bank ();
    ~Bank ();

    // Accessor Methods.
    int getAccountCounter () const;
    int getUserCounter () const;
    int getNumAccounts () const;
    int getNumUsers () const;
    Database<Account> getAccountDatabase () const;
    Database<User> getUserDatabase () const;
    User *getUser (int userId);
    Account *getAccount (int accountId);
    std::vector<ChequingAccount> getChequingAccounts () const;
    std::vector<SavingsAccount> getSavingsAccounts () const;
    std::vector<Manager> getManagers () const;
    std::vector<Customer> getCustomers () const;
    std::vector<Maintenance> getMaintenance () const;

    // Mutator Methods.
    void addChequing (int userId, double balance = 0, std::string name =
            "");
    void addSavings (int userId, double balance = 0,
            std::string name = "");
    void addCustomer (std::string firstName, std::string lastName,
            std::string password, std::string username);
    void addManager (std::string firstName, std::string lastName,
            std::string password, std::string username);
    void addMaintenance (std::string firstName, std::string lastName,
            std::string password, std::string username);
    void deleteAccount (int accountId);
    void deleteUser (int userId);
};

I have a template database class as such, which creates the vector of pointers:

template<class T>
    class Database
    {
        protected:
            std::vector<T*> m_database;
        public:
            Database ();
            virtual ~Database ();
            std::vector<T*> getDatabase () const;
            void add (T *Object);
            bool del (int id);
    };

In my bank, I am adding Account objects, which are defined as such:

class Account
{
    protected:
        int m_id, m_userId, m_type;
        double m_balance;
        std::string m_name;
        std::vector<std::string> m_history;
public:
    Account (int id, int userId, double balance = 0,
            std::string name = "");
    virtual ~Account ();

    // Accessor Methods.
    int getId () const;
    int getUserId () const;
    int getType () const;
    double getBalance () const;
    std::string getName () const;
    std::string getDetails () const;
    std::vector<std::string> getHistory () const;

    // Mutator Methods.
    void setName (std::string name);
    void depositFunds (double amount);
    virtual bool withdrawFunds (double amount);
};

/*
 * Chequing account type
 */
class ChequingAccount : public Account
{
    public:
        ChequingAccount (int id, int userId, double balance = 0,
                std::string name = "") :
                Account(id, userId, balance, name)
        {
            // Chequing account type.
            m_type = CHEQ;

            // If no name was given.
            if (name == "")
            {
                // Give account a generic name.
                m_name = "Chequing Account #" + std::to_string(id);
            }
            else
            {
                m_name = name;
            }
        }

        ~ChequingAccount ()
        {
        }

        virtual bool withdrawFunds (double amount);
};

/*
 * Savings account type
 */
class SavingsAccount : public Account
{
    public:
        SavingsAccount (int id, int userId, double balance, std::string name) :
                Account(id, userId, balance, name)
        {
            // Savings account type.
            m_type = SAVE;

            // If no name was given.
            if (name == "")
            {
                // Give account a generic name.
                m_name = "Savings Account #" + std::to_string(id);
            }
            else
            {
                m_name = name;
            }
        }

        ~SavingsAccount ()
        {
        }
};

I am adding the Account objects, primarily Chequing/Savings Accounts that are derived classes from the base Account class, to the Bank as such:

/*
 * Adds a new chequing account to the bank database.
 */
void Bank::addChequing (int userId, double balance, std::string name)
{
    m_acctDb.add(new ChequingAccount(++m_accountCounter, userId, balance, name));
}

/*
 * Adds a new savings account to the bank database.
 */
void Bank::addSavings (int userId, double balance, std::string name)
{
    m_acctDb.add(new SavingsAccount(++m_accountCounter, userId, balance, name));
}

This all works properly, and I am able to pull objects out of the database and manipulate how I like to. The issue is with the delete, which is defined as such:

/*
 * Deletes the specified account from the bank database.
 */
void Bank::deleteAccount (int accountId)
{
    std::vector<Account*> db = m_acctDb.getDatabase();
    std::vector<Account*>::iterator it = db.begin();
    cout << "Searching for account " << accountId << endl;
    while (it != db.end())
    {
        if ((*it)->getId() == accountId)
        {
            cout << "Found account 1" << endl;
            // Delete selected account.
            delete (*it);
            it = db.erase(db.begin());
        }
        else ++it;
    }
}

I created a small test file to test all the functionality:

int main ()
{
    Bank bank;

    cout << "Num accounts in bank: " << bank.getNumAccounts() << endl << endl;

    cout << "Adding accounts to bank..." << endl;
    bank.addChequing(1, 1500.0, "testchq");
    bank.addSavings(1, 2000.0, "testsav");

    cout << "Num accounts in bank: " << bank.getNumAccounts() << endl;
    for (int i = 0; i < bank.getNumAccounts(); ++i)
    {
        if (bank.getAccount(i + 1) == NULL) cout << "Account is NULL" << endl;
        else
        {
            cout << bank.getAccount(i + 1)->getDetails() << endl;
        }
    }
    cout << endl;

    cout << "Deleting account 1..." << endl;
    bank.deleteAccount(1);
    cout << endl;

    cout << "Num accounts in bank: " << bank.getNumAccounts() << endl;
    for (int i = 0; i < bank.getNumAccounts(); ++i)
    {
        if (bank.getAccount(i + 1) == NULL) cout << "Account is NULL" << endl;
        else
        {
            cout << bank.getAccount(i + 1)->getDetails() << endl;
        }
    }
}

and here is the output I get after running the file:

Num accounts in bank: 0

Adding accounts to bank...
Num accounts in bank: 2
Account #1 [C] testchq $1500.000000
Account #2 [S] testsav $2000.000000

Deleting account 1...
Searching for account 1
Found account 1

Num accounts in bank: 2
Account #1 [C] [C] $1500.000000
Account #2 [S] testsav $2000.000000

As you can see, it adds the derived Account classes properly, and retains their derived types without object slicing. In the delete function, you can see the delete function is finding the account it is supposed to delete properly. The problem is that while it should have deleted Account #1, it did not, but it did delete the account's name (as seen by Account #1 [C] [C] $1500.000000 vs Account #1 [C] testchq $1500.000000).

What is the problem I am experiencing here? I am also unsure about my method of doing this, so any suggestions for improvement would be greatly appreciated.

Thanks in advance!

1

There are 1 answers

4
Barry On BEST ANSWER

You're deleting the account from a copy:

std::vector<T*> getDatabase () const;

std::vector<Account*> db = m_acctDb.getDatabase();

You need to delete from the actual database, so you want the usage to be:

std::vector<T*>& getDatabase ();
const std::vector<T*>& getDatabase () const;

std::vector<Account*>& db = m_acctDb.getDatabase();