Hey guys, I am writing a program which uploads a text file into a linked list then displays a menu prompt to the user. All of my functions work fine except when I try to exit the menu/program. The program crashes and I don't see where it is coming from. Please help me to see and understand why my program is behaving as such. Here is the menu function:

void menuSelection(BankAccounts h)
{
    int selection;

    cout << "=============== Menu ===============" << endl;
    cout << "1) Show current balance" << endl; 
    cout << "2) Deposit" << endl;
    cout << "3) Withdrawal" << endl;
    cout << "4) Display All Accounts" << endl;
    cout << "5) Exit" << endl;
    cout << "Menu Selection: ";
    cin >> selection;

    while( cin.fail() || selection < 1 || selection > 5)
    {
        cin.clear();
        cin.ignore(10, '\n');
        cout << "Error: Invalid menu selection" << endl;
        cout << "Menu Selection: ";
        cin >> selection;
    }

    if (selection == 1)
    {
        h.showMyBalance();
        menuSelection(h);
    }
    else if (selection == 2)
    {
        h.makeDeposit();
        menuSelection(h);
    }
    else if (selection == 3)
    {
        h.makeWithdrawal();
        menuSelection(h);
    }
    else if (selection == 4)
    {
        h.showAll();
        menuSelection(h);
    }
    else if (selection == 5)
    {
        cout << "Have a nice day" << endl << endl;
    }
}

and here is how it is called in main()

int main()
{
    BankAccounts h;
    h.loadDataBase();
    menuSelection(h);

    return 0;
}

why is it when I choose 5, that the program crashes instead of passing control back to main and executing the next instruction which would be return 0? All other menu selections work without any errors.

Thanks for taking the time to read this, and thank you for your help in advance!

Oh, and if I make a selection of 4, but only 4 before I select 5, the program DOES NOT crash. Selecting any other option prior to 5 invokes a crash

h is created as you see it in main. BankAccounts class looks as so:

class BankAccounts
{
private:
    struct AccountsNode
    {
        int accountNo;
        string name;
        float accountBalance;
        AccountsNode *link;
    };

    AccountsNode *head, *newNode;
public:
    BankAccounts();
    void loadDataBase();
    friend void menuSelection(BankAccounts);
    void showMyBalance();
    void makeDeposit();
    void makeWithdrawal();
    void showAll();
    ~BankAccounts();
};

I still don't understand why it crashes on selection 5 though.

Does the program crash if you select option 5 without selecting any other option?

The code you have posted does not contain any obvious errors which means the error is else where. This is not entirely unusual, if some sort of undefined behaviour has been invoked it would be quite normal for the crash to happen in a place in the code completely apart from the location of the error.

A ha, I've spotted the error.

You are passing BankAccounts into menuSelection by value. Assuming that your BankAccounts::~BankAccounts dealloctates all the data and given that BankAccounts does not have a copy constructor and given the recursive way you call menuSelection rather than using a loop an iterating that means that the first time you call menuSelection it copies h. You select a menu option, it performs the function and calls menuSelection again copying h again. When you exit it deletes h deleting the data that it uses but because you have no copy constuctor for BackAccounts that is the same data that all the copies use to. When you exit (option 5) menuSelection and the local copy of h is deleted trying to deleting the internal data which has already been deleted causing the crash.

That it randomly works for option 4 is a random artifact of the undefined behaviour of deleting the same memory twice.

You need to do the following

  1. Pass BackAccounts to menuSelection by reference
  2. Do not call menuSelection recursively, ultimately you will always run out of stack space in the model you are using, use an iterative method (i.e. one that loops until exit time)
  3. Provide a proper copy constructor for BankAccounts so that if you do copy it it does a full copy not a shallow copy

Oh ok I see what you're saying. I actually removed the #5 option and instead replaced the entire option with a tryAgain() function. This way its not recursive and I can use my destructor to delete the memory by new pointer. Thank you for clarifying Banfa

This question has already been answered. Start a new discussion instead.