Banking system in C++

Question

I've created a program for Banking System in C++14.

You are given 9 options:

1. Open Account
2. Close Account
3. Show All Accounts
4. Deposit in Account
5. Withdraw from Account
6. Transfer to an Account
7. Show Balance
8. Show All Transactions
9. Quit

My program stores the deposits, and withdrawals, and transfers in a a file and uses them when the program is run again.

Note: The currency is in Indian rupees.

#include <string>
#include <vector>
#include <fstream>
#include <iostream>

using namespace std;

int MAX = 9999999999;

class Account
{
private:
    static int CurAccNo;

    bool open = true;
    int AccNo = ++CurAccNo;
    double balance = 0;
    string gender = "N/A";
    string first_name, second_name;

public:
    Account(){}
    Account(string, string, int);

    void deposit(double);
    void withdraw(double);
    void transfer(Account, double);

    static void setCurAccNo(int No){
        if(No > 0)
            CurAccNo = No;
    }

    int getAccNo();
    double getBalance();
    string getGender();
    string getFirstName();
    string getSecondName();

    void setBalance(double);
    void setGender(string);
    void setFirstName(string);
    void setSecondName(string);

    void close();
    bool isOpen();

    friend istream& operator >> (istream& in, Account &acc);
    friend ostream& operator << (ostream& out, Account &acc);

    friend ifstream& operator >> (ifstream& in, Account &acc);
    friend ofstream& operator << (ofstream& out, Account &acc);
};

int Account::CurAccNo = 0;

Account::Account(string first_name, string second_name, int initial_balance)
{
    setFirstName(first_name);
    setSecondName(second_name);
    balance = initial_balance;
}

void Account::deposit(double value){balance += value;}
void Account::withdraw(double value){balance -= value;}
void Account::transfer(Account other, double value){balance -= value ;other.balance += value;}

int Account::getAccNo(){return AccNo;}
double Account::getBalance(){return balance;}
string Account::getGender(){return gender;}
string Account::getFirstName(){return first_name;}
string Account::getSecondName(){return second_name;}

void Account::setBalance(double balance){if(balance >= 0) this -> balance = balance;}
void Account::setGender(string gender){if(gender == "M" || gender == "F") this -> gender = gender;}
void Account::setFirstName(string first_name){if(first_name != "") this -> first_name = first_name;}
void Account::setSecondName(string second_name){if(second_name != "") this -> second_name = second_name;}

void Account::close(){open = false;}
bool Account::isOpen(){return open;}

istream& operator >> (istream& in, Account &acc){
    string first_name, second_name;
    string gender;
    double balance;

    cout << "Enter First Name: "; cin >> first_name;
    cout << "Enter Second Name: "; cin >> second_name;
    cout << "Enter Gender (M / F): "; cin >> gender;
    cout << "Enter Initial Balance: "; cin >> balance;

    acc.setFirstName(first_name);
    acc.setSecondName(second_name);
    acc.setGender(gender);
    acc.setBalance(balance);

    return in;
}

ostream& operator << (ostream& out, Account &acc){
    out << "First Name: " << acc.getFirstName() << endl;
    out << "Second Name: " << acc.getSecondName() << endl;
    out << "Gender: " << acc.getGender() << endl;
    out << "Current Balance: " << acc.getBalance() << endl;
    out << "Account Number: " << acc.getAccNo() << endl;
    out << "Status: " << (acc.isOpen()? "Open" : "Closed") << endl;

    return out;
}

ifstream& operator >> (ifstream& in, Account &acc){
    string first_name, second_name;
    string gender;
    int AccNo; bool open;
    double balance;

    in >> first_name >> second_name;
    in >> gender;
    in >> balance >> AccNo;
    in >> open;

    acc.setFirstName(first_name);
    acc.setSecondName(second_name);
    acc.setGender(gender);
    acc.setBalance(balance);
    acc.open = open;
    acc.AccNo = AccNo;

    return in;
}

ofstream& operator << (ofstream& out, Account &acc){
    out << acc.getFirstName() << endl;
    out << acc.getSecondName() << endl;
    out << acc.getGender() << endl;
    out << acc.getBalance() << endl;
    out << acc.getAccNo() << endl;
    out << acc.isOpen();

    return out;
}

bool isEmpty(ifstream& pFile)
{
    return pFile.peek() == ifstream::traits_type::eof();
}

bool Valid(unsigned int AccNo, vector<Account> Accounts){
    if(AccNo <= Accounts.size() && AccNo > 0 && Accounts[AccNo].isOpen())
        return true;
    return false;
}

int main()
{
    vector<Account> Accounts;

    ofstream otrans("Transactions.txt", ios::app);

    ofstream ofs("BankingSystem.txt", ios::app);
    ifstream ifs("BankingSystem.txt");

    vector<int> AccNos;
    vector<string> types;
    vector<double> values;

    while(!ifs.eof() && !isEmpty(ifs)){
        Account acc;
        ifs >> acc;

        Accounts.push_back(acc);
    }

    Account::setCurAccNo(Accounts.size());

    while(true){
        cout << "|=============================|" << endl;
        cout << "|       WELCOME TO            |" << endl;
        cout << "|          RANDOM BANK        |" << endl;
        cout << "|=============================|" << endl;

        cout << endl;

        cout << "What Would You Like To Do?:" << endl;
        cout << "   1. Open Account" << endl;
        cout << "   2. Close Account" << endl;
        cout << "   3. Show All Accounts" << endl;
        cout << "   4. Deposit in Account" << endl;
        cout << "   5. Withdraw in Account" << endl;
        cout << "   6. Transfer to an Account" << endl;
        cout << "   7. Show Balance" << endl;
        cout << "   8. Show All Transactions" << endl;
        cout << "   9. Quit" << endl;
        cout << endl;

        string option;
        cout << "Please Enter Your Option: "; cin >> option;

        if(option == "1"){
            Account acc;
            cin >> acc;

            if(Accounts.size() != 0)
                ofs << endl;

            Accounts.push_back(acc);
            ofs << acc;

            cout << "Account Opened Successfully!" << endl;
            cout << "Your Account Number is " << acc.getAccNo() << endl;
        }

        else if(option == "2"){
            int AccNo;
            cout << "Enter Account Number: "; cin >> AccNo;

            AccNo--;

            if(Valid(AccNo, Accounts)){
                cout << "Account Closed Successfully!" << endl;
                Accounts[AccNo].close();
            }
            else
                cout << "There Was an Error, Please Try Again!" << endl;
        }

        else if(option == "3"){
            cout << endl;

            for(unsigned int i = 0; i < Accounts.size(); i++)
                cout << Accounts[i] << endl;
        }

        else if(option == "4"){
            int AccNo;
            double value;
            cout << "Enter Account Number: "; cin >> AccNo;
            cout << "Enter Deposit Amount: "; cin >> value;

            AccNo--;

            if(Valid(AccNo, Accounts)){
                if(value < MAX){
                    cout << value << " Deposited to " << Accounts[AccNo].getFirstName() << "'s Account Successfully!" << endl;

                    Accounts[AccNo].deposit(value);
                    cout << "Balance is " << Accounts[AccNo].getBalance() << endl;

                    AccNos.push_back(AccNo);
                    types.push_back("Deposit");
                    values.push_back(value);
                }
                else
                    cout << "Overflow!" << endl;
            }
            else
                cout << "There Was an Error, Please Try Again!" << endl;
        }

        else if(option == "5"){
            int AccNo;
            double value;
            cout << "Enter Account Number: "; cin >> AccNo;
            cout << "Enter Withdrawal Amount: "; cin >> value;

            AccNo--;

            if(Valid(AccNo, Accounts)){
                if(Accounts[AccNo].getBalance() - value >= 0){
                    cout << value << " Withdrawn from " << Accounts[AccNo].getFirstName() << "'s Account Successfully!" << endl;

                    Accounts[AccNo].withdraw(value);
                    cout << "Balance is " << Accounts[AccNo].getBalance() << endl;

                    AccNos.push_back(AccNo);
                    types.push_back("Withdrawal");
                    values.push_back(value);
                }
                else
                    cout << "Not Enough Balance" << endl;
            }
            else
                cout << "There Was an Error, Please Try Again!" << endl;
        }

        else if(option == "6"){
            int AccNo, AccNoOther;
            double value;
            cout << "Enter Account Number: "; cin >> AccNo;
            cout << "Enter Other Account Number: "; cin >> AccNoOther;
            cout << "Enter Transfer Amount: "; cin >> value;

            AccNo--;

            if(Valid(AccNo, Accounts) && Valid(AccNoOther, Accounts)){
                if(Accounts[AccNoOther].getBalance() + value > MAX)
                    cout << "Overflow" << endl;
                else {
                    if(Accounts[AccNo].getBalance() - value >= 0){
                        cout << value << " Transfer to " << Accounts[AccNoOther].getFirstName() << "'s Account Successfully!" << endl;

                        Accounts[AccNo].transfer(Accounts[AccNoOther], value);
                        cout << Accounts[AccNo].getFirstName() << "'s Balance is " << Accounts[AccNo].getBalance() << endl;
                        cout << Accounts[AccNoOther].getFirstName() << "'s Balance is " << Accounts[AccNo].getBalance() << endl;
                    }

                    else
                        cout << "Not Enough Balance" << endl;
                }
            }

            else
                cout << "There Was an Error, Please Try Again!" << endl;
        }

        else if(option == "7"){
            int AccNo;
            cout << "Enter Account Number: "; cin >> AccNo;

            AccNo--;

            cout << endl;

            if(Valid(AccNo, Accounts))
                cout << Accounts[AccNo];
            else
                cout << "There Was an Error, Please Try Again!" << endl;
        }

        else if(option == "8"){
            int AccNo;
            cout << "Enter Account Number: "; cin >> AccNo;

            AccNo--;

            if(Valid(AccNo, Accounts)){
                ifstream itrans("Transactions.txt");
                vector<double> deposits;
                vector<double> withdrawals;

                string x = "Deposits";

                while(!itrans.eof() && !isEmpty(itrans)){
                    int TransAcc;
                    string type;
                    double value;

                    itrans >> TransAcc >> type >> value;

                    if(TransAcc == AccNo){
                        if(type == "Deposit")
                            deposits.push_back(value);
                        if(type == "Withdrawal")
                            withdrawals.push_back(value);
                    }
                }

                for(unsigned int i = 0; i < AccNos.size(); i++){
                    int TransAcc = AccNos[i];
                    string type = types[i];
                    double value = values[i];

                    if(TransAcc == AccNo){
                        if(type == "Deposit")
                            deposits.push_back(value);
                        if(type == "Withdrawal")
                            withdrawals.push_back(value);
                    }
                }

                cout << "=======================================" << endl;
                cout << "Withdrawal         |           Deposits" << endl;
                cout << "=======================================" << endl;
                cout << endl;

                for(unsigned int i = 0; i < max(deposits.size(), withdrawals.size()); i++){
                    int s = 0;

                    if(i < withdrawals.size()){
                        string x = to_string(withdrawals[i]);
                        cout << x;

                        s = x.size();
                    }

                    string x = "Deposits           ";

                    for(unsigned int _ = 0; _ < x.size() - s; _++)
                        cout << " ";

                    cout << "|           ";

                    if(i < deposits.size()){
                        string x = to_string(deposits[i]);
                        cout << x;
                    }

                    cout << endl;
                }
            }
            else
                cout << "There Was an Error, Please Try Again!" << endl;
        }

        else if(option == "9"){
            break;
        }

        else {
            cout << "Please Enter a Valid Option" << endl;
        }

        cout << endl;

        system("pause");
        system("cls");
    }

    ofstream ofsn("BankingSystem.txt");

    for(unsigned int i = 0; i < Accounts.size(); i++){
        if(i != 0)
            ofsn << "\n";
        ofsn << Accounts[i];
    }

    for(unsigned int i = 0; i < AccNos.size(); i++){
        if(i != 0)
            otrans << "\n";
        otrans << AccNos[i] << " " << types[i] << " " << values[i] << endl;
    }
}

It works as expected to my knowledge. I would like to improve this as much as possible. Also, tips for adding more functions to the System are welcome.

Answer 1

Overview

The code is OK.

I found 1 bug.

The encapsulation is slightly lacking. You have Account as a class, which is good, but the getters/setters are not required and you have better methods to interact with it.

On encapsulation, you need to encapsulate a set of accounts and the "next account number" into a class (maybe Bank). Currently, you set up a vector of accounts and then have to remember to set the next highest account number back in Account.

I dislike the way you get user interaction inside a stream operator (input). I would change that and use stream operators simply for that.

Your main() needs to be broken up a bit to make the code a bit more self-documenting.

I dislike how user input of the account number requires you to then subtract one from the account number before you can use it. This is going to lead to a maintainability error where somebody forgets to subtract one before using the value entered by the user.

Code Review

Using namespace std is bad practice.

using namespace std;

See: Why is “using namespace std;” considered bad practice?

The answers to this question go into a lot of detail about the subject. It is definitely a habit that is hard to break. But one that you should (as detailed in the answers to the above question).

But I hear you ask "Why do all books/articles always start with: using namespace std;?

The answer is context. In a book and/or magazine article the primary purpose is not maintainability but space (or the lack of it). They are prioritizing for space while in real life (or a professional setting) we are prioritizing for maintainability.

---

Using Global Constant rather than define

This is a good start.

int MAX = 9999999999;

But from the context, it also looks like it should be const. Or if you have a C compiler that supports more modern language, constexpr.

int constexpr MAX = 9999999999;

In general, any globally accessible state should be non-mutable (unless you have a very good reason for it not to be).

---

Readability:

    string first_name, second_name;

Please, one variable per line. I have not come across a coding standard that does not emphasize this. We are not trying to win competitions on the least vertical space we can use. The whole point of using a high-level language is easier maintainability. One variable per line will help with that.

    std::string     first_name;
    std::string     second_name;

---

Avoid getter/setter

    int getAccNo();
    double getBalance();
    string getGender();
    string getFirstName();
    string getSecondName();

    void setBalance(double);
    void setGender(string);
    void setFirstName(string);
    void setSecondName(string);

These break encapsulation.

Yes sure, it is better than making the variables public. But what are you trying to archive? Getters and setters are good for property bags (property bag => your class is just a set of unrelated objects being transported together) not for class objects where the state represents a thing.

    a.setName(a.getName() + " Tool"); // Is this a valid use case?

If you have a function that retrieves the state, modifies the state, then puts the new state back, should that functionality not be a part of the class? This is why methods tend to be actions that are applied to the object.

Like these methods:

    void deposit(double);
    void withdraw(double);
    void transfer(Account, double);

These methods are great.

If we analyze why you have getters and setters:

Methods used for printing:

 getBalance
 getGender
 getFirstName
 getSecondName

Methods used for passing in data:

 setBalance
 setGender
 setFirstName
 setSecondName

Validating that an amount can be added/withdrawn:

 getBalance

Used in constructor to validate input:

 setBalance
 setFirstName
 setSecondName

So you have 8 functions but only 4 primary use cases. The validation done by the constructor is a bit primitive and may as well be done by the constructor. The output operator (printing should be a friend anyway) is part of the public API for the Account class. The input operator similarly should be part of the public API but can more efficiently use the constructor and a swap operator to achieve the same results.

So I think the only valid use case is 'Validating that an amount can be added/withdrawn', which deserves its own method to make it clear what you are doing and the result of the action.

I'll go into more detail below when I re-design the class. See below.

---

Avoid silently failing checks

The following appears to fail if no valid number is passed in:

    static void setCurAccNo(int No){
        if(No > 0)
            CurAccNo = No;
    }

Silent failure is the enemy of all coders. Make this scream and holler on failure as something has gone wrong. I would throw an exception on failure to make the application quit.

---

Currency should be integers

When talking about representing money, doubles are a terrible way to go.

    double getBalance();

The trouble is that they have rounding errors. You should use an integer value. If this is for American currency with dollars and cents then record the number of cents. When you display it you can convert to dollars by dividing by 100. But never store currency in a double.

---

Enum when you have a small subset of valid values

Is a gender a string?

    string getGender();

Maybe this is my old world bias showing here. I am assuming a small number of known versions.

What are you trying to store and why? Is it to help with fraud detection?

The reason I would not use a string is that it allows too many variants which could look valid under programmer scrutiny but are not actually valid inputs for the system.

 "f" or "F" or "Female" or "female" or "FEMALE"
 "m" or "M" or "Male" or "male" or "MALE" or 
 etc

These would all look valid to casual inspection. I think a better option would be an enum.

On the counter side of the argument would be the ability of the system to adapt to alternative gender types that had not been considered at the time the application was first written.

---

Stream formatting

You want a different type of streaming to normal streaming when printing to a file.

    friend istream& operator >> (istream& in, Account &acc);
    friend ostream& operator << (ostream& out, Account &acc);

    friend ifstream& operator >> (ifstream& in, Account &acc);
    friend ofstream& operator << (ofstream& out, Account &acc);

This effect is normally achieved with formatters. The account object would always be treated the same when printed to a stream. Because just because what you have locally is a reference to a stream does not mean it is not a file stream. This decision is made at compile time (not runtime). So you can definitely get different behaviors than expected.

To solve for this, you create "Format" objects which know how to format an account for a stream. You may have a "pretty print" format or a "pine print" format for printing to different types of forms, etc.

 std::cout << PrettyPrintFormat(acc) << "\n";
 file      << LinePrintFormat(acc) << "\n";

---

Bug

Bug:

void Account::transfer(Account other, double value)
{
    balance -= value ;
    other.balance += value;
}

You are passing the account by value.
You have modified the copy of the account, not the original account.

---

Don't use interactions when streaming.

Asking for user interaction as part of the streaming processes?

istream& operator >> (istream& in, Account &acc){
    string first_name, second_name;
    string gender;
    double balance;

    cout << "Enter First Name: "; cin >> first_name;
    cout << "Enter Second Name: "; cin >> second_name;
    cout << "Enter Gender (M / F): "; cin >> gender;
    cout << "Enter Initial Balance: "; cin >> balance;

    acc.setFirstName(first_name);
    acc.setSecondName(second_name);
    acc.setGender(gender);
    acc.setBalance(balance);

    return in;
}

---

Evil

Dastardly

bool isEmpty(ifstream& pFile)
{
    return pFile.peek() == ifstream::traits_type::eof();
}

You are subverting the primary pattern used by all other developers. This is going to lead to errors in the long run. Please stick to established patterns.

This is where your dastardly pattern is used.

    while(!ifs.eof() && !isEmpty(ifs)){
        Account acc;
        ifs >> acc;

        Accounts.push_back(acc);
    }

It doesn't read that poorly, but it still makes it look wrong as most people are not expecting isEmpty() to peek. As such, you are going to go make them check. But it still looks better if you use the normal pattern:

    Account acc;
    while(ifs >> acc) {
        Accounts.push_back(acc);
    }

Also you can take this a step further. You don't even need the loop. Just initialize the account vector with iterators:

    std::ifstream        ifs("BankingSystem.txt");
    std::vector<Account> Accounts(std::istream_iterator<Account>{ifs}, std::istream_iterator<Account>{});

Done. Standard patterns are nice.

---

return bool

The use of an if/then to return a bool.

bool func() {
    if (cond) {return true;}
    else      {return false;}
}

Can be simplified to:

bool func {return cond;}

So you can simplify this:

bool Valid(unsigned int AccNo, vector<Account> Accounts){
    if(AccNo <= Accounts.size() && AccNo > 0 && Accounts[AccNo].isOpen())
        return true;
    return false;
}

---

Encapsulation

Hmm:

    Account::setCurAccNo(Accounts.size());

Forcing initialization of some global state independently of loading the state. This is a bug waiting to happen. Your account state and the next number need to be combined into their own class.

    class Bank
    {
          // Some accounts.
          // Some state about the next valid bank account number.
    };

There is a dependency between Accounts and Account::CurAccNo you need to formalize and protect this dependency.

---

Prefer "\n" over std::endl

Prefer "\n" over std::endl.

    while(true){
        cout << "|=============================|" << endl;
        cout << "|       WELCOME TO            |" << endl;
        cout << "|          RANDOM BANK        |" << endl;
        cout << "|=============================|" << endl;

        cout << endl;

Did you really want to force a flush after each line? Why? Don't do it. This is what leads to all the slowdowns in the C++ streams unnecessarily flushed by the engineer. The streams will always flush themselves at the correct time. You should only force a flush if you need to (unless you are very experienced and have done the tests to prove it).

---

Self-documenting code

Long switch (or multi-branch if/else) statements are a pain to read.

This is where self-documenting code comes in. Each switch can call a function with a nice name so that you know what the option is doing.

        if(option == "1"){
            Account acc;
            cin >> acc;

            if(Accounts.size() != 0)
                ofs << endl;

            Accounts.push_back(acc);
            ofs << acc;

            cout << "Account Opened Successfully!" << endl;
            cout << "Your Account Number is " << acc.getAccNo() << endl;
        }
        etc...

        // More like this:

        switch(option) {
            case 1: loadAccount(); break;
            case 2: closeAccount(); break;
            etc...

---

Notes

        if(option == "1"){

This seems like the perfect place to ask those questions you put in the input operator. Account acc;
cin >> acc;

---

Range-based for loop

        else if(option == "3"){
            cout << endl;

            for(unsigned int i = 0; i < Accounts.size(); i++)
                cout << Accounts[i] << endl;

You might want to instead look at using a range-based for loop:

            for(auto const& account: Accounts) {
                std::cout << account << "\n";
            }

---

Alternative

#include <iomanip>
#include <iostream>
#include <fstream>
#include <vector>
#include <iterator>

enum Action {BADAction, OpenAccount, CloseAccount, ShowAllAccounts, DepositInAccount, WithdrawFromAccount, TransferToAnAccount, ShowBalance, ShowAllTransactions, Quit};

std::istream& operator>>(std::istream& str, Action& action)
{
    int value;
    if (str >> value && value >= 1 && value <= 9) {
        action = static_cast<Action>(value);
    }
    else {
        action = BADAction;
    }
    return str;
}
std::ostream& operator<<(std::ostream& str, Action const& action)
{
    return str << static_cast<int>(action);
}

class AccountPrettyPrint;
class Account
{
    // Pretty printing is tightly coupled to the
    // state of the class. So we are noting this
    // by making it a friend of the class.
    friend class AccountPrettyPrint;

    std::string         firstName;
    std::string         surName;
    int                 balance;
    bool                open;
    int                 accountNo;

    public:
        Account();
        Account(std::string const& firstName, std::string const& surName, int balance);
        Account(Account const&)             = default;
        Account(Account&&)                  = default;
        Account& operator=(Account const&)  = default;
        Account& operator=(Account&&)       = default;

        void swap(Account& other) noexcept;
        friend void swap(Account& lhs, Account& rhs) {lhs.swap(rhs);}
        friend std::ostream& operator<<(std::ostream& str, Account const& acc);
        friend std::istream& operator>>(std::istream& str, Account& acc);
};

struct AccountPrettyPrint
{
    Account const&    acc;
    AccountPrettyPrint(Account const& acc): acc(acc) {}

    std::ostream& print(std::ostream& str) const
    {
        return str  << "Print Account Nicely: " << acc.firstName << "\n"
                    << " $" << (acc.balance / 100)
                    << "." << std::setw(2) << std::setfill('0') << (acc.balance % 100)
                    << "\n";
    }
    friend std::ostream& operator<<(std::ostream& str, AccountPrettyPrint const& pp)
    {
        return pp.print(str);
    }
};

class Bank
{
    static std::vector<Account>        accounts;
    public:
        static int getNextAccountNumber()   {return accounts.size();}
        static void loadAccount(std::string const& fileName);
};

Account::Account()
    : balance(0)
    , open(false)
    , accountNo(-1)
{}

Account::Account(std::string const& firstName, std::string const& surName, int balance)
    : firstName(firstName)
    , surName(surName)
    , balance(balance)
    , open(true)
    , accountNo(Bank::getNextAccountNumber())
{}

void Account::swap(Account& other) noexcept
{
    using std::swap;
    swap(firstName, other.firstName);
    swap(surName,   other.surName);
    swap(balance,   other.balance);
    swap(open,      other.open);
    swap(accountNo, other.accountNo);
}

std::ostream& operator<<(std::ostream& str, Account const& acc)
{
    return str  << acc.firstName.size() << " " << acc.firstName
                << acc.surName.size() << " " << acc.surName
                << acc.balance << " "
                << acc.open << " "
                << acc.accountNo << "\n";
}

std::istream& operator>>(std::istream& str, Account& acc)
{
    Account     tmp;
    std::size_t size;
    char        ignore;
    if (str >> size) {
        tmp.firstName.resize(size);
        str.read(&ignore, 1);
        str.read(&tmp.firstName[0], size);
    }
    if (str >> size) {
        tmp.surName.resize(size);
        str.read(&ignore, 1);
        str.read(&tmp.firstName[0], size);
    }
    if (str >> tmp.balance >> tmp.open >> tmp.accountNo) {
        // Only change the state if the object was correctly read from the stream.
        acc.swap(tmp);
    }
    return str;
}

std::vector<Account> Bank::accounts;

void Bank::loadAccount(std::string const& fileName)
{
    std::ifstream           logFile(fileName);
    std::vector<Account>    load(std::istream_iterator<Account>{logFile},
                                 std::istream_iterator<Account>{});

    accounts = std::move(load);
}
oid reportError()
{
    std::cerr << "Unrecognized user input\n";
    std::cerr << "Flushing Input stream\n";
    std::cin.clear();
    cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
}
void openAccount()
{
    // Get user input
    // Bank::openAccount(userInput);
}
void closeAccount()
{
    // get user input
    // Bank::closeAccount(userInput);
}
void showAllAccounts()
{
    // Bank::showBalance()
}
void depositInAccount()
{
    // getUserInput
    // Bank::deposit(account, value)
}
void withdrawFromAccount()
{
    // get user input
    // Bank::withdraw(account, value);
}
void transferToAnAccount()
{
    // get user input
    // Bank::transfer(srcAccount, dstAccount, value);
}
void showBalance()
{
    // get user input
    // Bank::showBalance(account);
}
void showAllTransactions()
{
    // Bank::showAllTransactions();
}

void displayMenu()
{
    std::cout << "Choose an Option:\n"
              << "\t" << OpenAccount            << " To open an Account\n"
              << "\t" << CloseAccount           << " To close an Account\n"
              << "\t" << ShowAllAccounts        << " To show all accounts\n"
              << "\t" << DepositInAccount       << " To deposit in an account\n"
              << "\t" << WithdrawFromAccount    << " To withdraw from an account\n"
              << "\t" << TransferToAnAccount    << " To transfer to an account\n"
              << "\t" << ShowBalance            << " To show account balance\n"
              << "\t" << ShowAllTransactions    << " To show all transactions\n"
              << "\t" << Quit                   << " To quit application\n";
}

int main(int argc, char* argv[])
{
    if (argc != 2) {
        std::cerr << "Failed to start: CLA <transaction Log Name>\n";
        exit(1);
    }
    Bank::loadAccount(argv[1]);

    Action  userAction;
    do {
        displayMenu();
        std::cin >> userAction;
        switch(userAction) {
            case BADAction:             reportError();          break;
            case OpenAccount:           openAccount();          break;
            case CloseAccount:          closeAccount();         break;
            case ShowAllAccounts:       showAllAccounts();      break;
            case DepositInAccount:      depositInAccount();     break;
            case WithdrawFromAccount:   withdrawFromAccount();  break;
            case TransferToAnAccount:   transferToAnAccount();  break;
            case ShowBalance:           showBalance();          break;
            case ShowAllTransactions:   showAllTransactions();  break;
            case Quit:                  /* Nothing */           break;
        }
    }
    while(userAction != Quit);
}

Answer 2

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Implement operator<< and operator>> regardless of terminal or file input

The only implementations you should have are

friend istream& operator >> (istream& in, Account &acc);
friend ostream& operator << (ostream& out, const Account &acc);
                                        // ^^^^^ Note the const here

Mixing the text extraction operators with std::cin as you do here

istream& operator >> (istream& in, Account &acc){
    string first_name, second_name;
    string gender;
    double balance;

    cout << "Enter First Name: "; cin >> first_name;
    cout << "Enter Second Name: "; cin >> second_name;
    cout << "Enter Gender (M / F): "; cin >> gender;
    cout << "Enter Initial Balance: "; cin >> balance;

    acc.setFirstName(first_name);
    acc.setSecondName(second_name);
    acc.setGender(gender);
    acc.setBalance(balance);

    return in;
}

restricts reusability and is a bad implementation.

Rather use the setters, or appropriate constructors for such case.

Also the implementation of friend ofstream& operator << (ofstream& out, const Account &acc);

is redundant. friend ostream& operator << (ostream& out, const Account &acc); will already work for any kinf of std::ostream.

Answer 3

Missing Header Include

This code should have

#include <algorithm>

So that the call to max() will compile.

Symbolic Constants

The declaration for MAX makes it a variable rather than a constant, that means that it could be changed within the code. It might be better if it was declared as a constant

constexpr int MAX = 9999999999;

so that the code can't change it.

Avoid using namespace std`

As stated in another review if you are coding professionally you should get out of the habit of using the using namespace std; statement. The code will more clearly define where cout and other identifiers are coming from (std::cin, std::cout). As you start using namespaces in your code it is better to identify where each function comes from because there may be function name collisions from different namespaces. The identifiercout you may override within your own classes, and you may override the operator << in your own classes as well.

Put Classes in Their Own Files

One of the basic reasons for Object Oriented Programming is that objects are reusable, however, if the object is defined in the same file as main() it can't be reused or shared between modules. Most C++ editors (IDEs) have a way to create classes that generate both a header file and a C++ source file for the class. Another reason for doing separating classes into their own files is that it makes building, writing, debugging, reading, maintaining and testing code easier. During build only files that have been edited will recompile.

Complexity

The function main() is too complex (does too much). As programs grow in size the use of main() should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program.

There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

There are at least 10 possible functions in main().

• Show Menu
• Get Option (calls show menu and returns the option)
• Open Account
• Close Account
• Deposit in Account
• Withdraw from Account
• Show All Accounts
• Transfer to an Account
• Show Balance
• Show All Transactions

Quit is not a function because it only indicates that the while loop should end.

Rather than use multiple else if to process the options, use a switch statement:

        int option;
        cout << "Please Enter Your Option Number: "; cin >> option;

        switch (option)
        {
            case 1: 
                OpenAccount();
                continue;
            case 2:
                CloseAccount();
                continue;
            case 3:
                ShowAllAccounts();
                continue;
                ...
            default:
                cerr << "Invalid option " << option << "please enter a valid option\n";
                continue;
        }

There are conceptually other classes that could help implement the program, such as class Bank which would contain the accounts and the current account number to assign to an account. The current account number to assign does not belong in the Account class. The list of transactions belongs in the Account class because the user should not see any transactions that do not belong to them. There should also be a Customer class that contains the customer data name and gender. A customer may have several accounts.

Default Constructors

Rather than creating the default constructor as Account(){}, C++ allows you to assign a default constructor Account() = default; This is more readable.

Readability

There are a number of this that would improve the readability of the code, which would not only make it easier to review, but easier to maintain the code as well.

• Put all declarations on separate lines.
• Horizontal spacing.
• Use code blocks even for single statements within if statements, then clauses and loops.

Answer 4

Never use float or double for money

Floating point types (in C++, those include float, double, long double, etc.) are inherently inaccurate due to how they are represented in memory. It's not a huge deal for most applications - but for a banking system (where rounding error is unacceptable) you should stay as far away as possible from this. A solution to this is to separate the whole and fractional amounts as integer types:

struct Money {
    int dollars = 0, cents = 0;
}

Then, define some operators:

Money operator+(const Money &x, const Money &y);
Money operator-(const Money &x, const Money &y);
...

Answer 5

No comments anywhere within the code. This could lead to the code being difficult to maintain/understand in the future.