An account/banking system in C++

Question

This is just a basic accounting/banking system with the user being able to register an account, check their balance, make a withdrawal and deposit money etc. There is a few things a staff account can do also such as the removal of an account.

AccountSystem.h

#pragma once

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>


enum type_acc {customer, staff};

class Account {
public:
    std::string username;
    std::string password;
    double balance; 
    type_acc account_type; 
    bool operator==(const Account& rhs);

};

class BankInterface {
public:
    static Account loggedinaccount;
    static std::vector<Account> registered_accounts; 
    static void LoginMenu();
    static void RegisterAccount(); 
    static void Login(); 
    static void MainMenu(); 
    static void DisplayBalance();
    static void MakeWithdrawal();
    static void MakeDeposit(); 
    static type_acc AccountType(std::string uc); 


};

class StaffInterface {
public:
    static void StaffMenu();
    static void CustomerBalance();
    static void AccountShutdown(); 
};

Account BankInterface::loggedinaccount; 
std::vector<Account> BankInterface::registered_accounts; 

AccountSystem.cpp

// AccountSystem.cpp : This file contains the 'main' function. Program execution begins and ends there.
//

#include "pch.h"
#include "AccountSystem.h"

void BankInterface::LoginMenu() {
    std::cout << "Please select an option: \n 1. Register a new account. \n 2. Login to account.\n";

    std::string user_choice;
    std::cin >> user_choice;

    if (user_choice == "1") {
        RegisterAccount();
    }
    else if (user_choice == "2") {
        Login(); 
    }

    else {
        std::cout << "We did not recognise your input, please try again...\n";
        LoginMenu(); 
    }
}

type_acc BankInterface::AccountType(std::string uc) {
    if (uc == "y") {
        return staff;
    }
    else if (uc == "n") {
        return customer;
    }
    else {
        std::cout << "We did not recognise your input, please try again.\n"; 
        std::string user_choice;
        std::cin >> user_choice;
        return AccountType(user_choice); 
    }
}

void BankInterface::RegisterAccount() {
    Account new_account;
    std::cout << "Please enter the desired username of your account: \n";
    std::cin >> new_account.username;
    std::cout << "Please enter your desired password for your account.\n";
    std::cin >> new_account.password; 
    std::cout << "Please enter your initial balance for your account.\n"; 
    std::cin >> new_account.balance; 
    std::cout << "Are you a member of staff? y/n?\n";
    std::string user_choice; 
    std::cin >> user_choice; 
    new_account.account_type = BankInterface::AccountType(user_choice); 
    registered_accounts.push_back(new_account);
    std::cout << "Account created.\n"; 

    for (auto &account : registered_accounts) {
        std::cout << account.username << "\n"; 
    }

    LoginMenu(); 

}

void BankInterface::Login() {
    std::cout << "Please enter your username.\n"; 
    std::string username;
    std::cin >> username; 

    for (auto &account : registered_accounts) {
        if (account.username == username) {
            std::cout << "Please enter your password.\n"; 
            std::string password;
            std::cin >> password; 
            if (account.password == password) {
                loggedinaccount = account; 

                if (loggedinaccount.account_type == customer) {
                    MainMenu();
                }
                else {
                    StaffInterface::StaffMenu(); 
                }

            }
        }

    }
    std::cout << "That username does not exist in our system.\n";
    Login();
}

void BankInterface::MainMenu() {
    std::cout << " 1. Check account balance.\n 2. Make a withdrawal.\n 3. Make a deposit. \n";

    int user_choice = 0; 
    std::cin >> user_choice; 

    switch (user_choice) {
    case 1: 
        DisplayBalance(); 
        break; 
    case 2:
        MakeWithdrawal(); 
        break; 
    case 3:
        MakeDeposit(); 
        break; 
    default:
        std::cout << "We did not understand your input, please try again.";
        MainMenu();
        break; 
    }

}

void BankInterface::DisplayBalance() {
    std::cout <<  "Your balance is: " << loggedinaccount.balance << "\n";
    MainMenu(); 
}

void BankInterface::MakeWithdrawal() {
    std::cout << "Please enter the amount you wish to withdraw: \n";
    double amt;
    std::cin >> amt; 

    if (amt > loggedinaccount.balance) {
        std::cout << "You do not have this much money in your account, please input a feasible sum to withdraw. \n"; 
        MakeWithdrawal(); 
    }
    else if (amt <= 0) {
        std::cout << "You cannot withdraw a negative amount, or zero, dollars, please try again.\n"; 
        MakeWithdrawal(); 
    }
    else {
        loggedinaccount.balance -= amt; 
        std::cout << "Money withdrawn, new balance: " << loggedinaccount.balance << "\n";
        MainMenu(); 
    }

}

void BankInterface::MakeDeposit() {
    std::cout << "How much money do you wish to deposit? \n";
    double amt;
    std::cin >> amt; 

    if (amt <= 0) {
        std::cout << "You cannot deposit a negative amount of, or zero, dollars. Please try again.\n";
        MakeDeposit(); 
    }
    else {
        loggedinaccount.balance += amt; 
        std::cout << "Your new balance is: " << loggedinaccount.balance << "\n";
        MainMenu(); 
    }

}

void StaffInterface::StaffMenu() {
    std::cout << "1. View customer's balance.\n2. Shut down customer's account.\n"; 

    int staff_choice = 0;
    std::cin >> staff_choice;

    switch (staff_choice) {
    case 1:
        CustomerBalance(); 
        break;
    case 2:
        AccountShutdown(); 
        break;
    default:
        std::cout << "We did not understand your input, please try again.";
        StaffMenu(); 
        break;
    }

}

void StaffInterface::CustomerBalance() {
    std::cout << "Enter the customer's username to view their balance: \n";
    std::string cust_name;
    std::cin >> cust_name;

    for (auto &account : BankInterface::registered_accounts) {
        if (cust_name == account.username) {
            std::cout << account.balance << "\n";
            StaffMenu(); 
        }
        else {
            std::cout << "We could not detect an account with that username, please try again.\n";
            CustomerBalance();
        }
    }
}

bool Account::operator==(const Account& rhs) {
    if (username == rhs.username) {
        return true;
    }
    else {
        return false; 
    }
}

void StaffInterface::AccountShutdown() {

    std::cout << "Enter the customer's username of the account you want to delete.\n";
    std::string username_del;
    std::cin >> username_del; 
    for (auto &account : BankInterface::registered_accounts) {
        if (username_del == account.username) {
            std::cout << "Account detected, are you sure you wish to delete this account? y/n \n";
            std::string confirmation; 
            bool ans = false;

            while (ans != true) {
                std::cin >> confirmation;

                if (confirmation == "y") {
                    auto pos = find(BankInterface::registered_accounts.begin(), BankInterface::registered_accounts.end(), account);
                    BankInterface::registered_accounts.erase(pos); 
                    std::cout << "Account deleted, returning to main staff menu. \n"; 
                    ans = true; 
                    StaffMenu(); 
                }
                else if (confirmation == "n") {
                    ans = true; 
                    StaffMenu(); 
                }
                else {
                    std::cout << "We did not recognise that option, please try again.\n";
                }

            }

        }
    }
}

int main()
{
    BankInterface::LoginMenu(); 
}

Any help/tips are appreciated.

Answer 1

Focusing just on non-functional aspects:

Encapsulation

It is considered very bad practice to have any static fields, as you have in your Account class, because then you have no control over their use. You should make these fields private, and provide whatever methods are necessary to access and update them.

Candidate for Polymorphism

You appear to have two different types of accounts, one used by customers and the other used by staff. At the moment they are being differentiated by a type code, and then different methods are being called based upon the account being used. With polymorphism, you can call a single object on some type of Account, without caring whether it is a customer or staff account, and the method of the correct class will be invoked.

Endless Recursive Loops

If this application is left open too long, your call stack will grow indefinitely. This introduces the risk that other data, possibly in other programs, could be corrupted. Rather than calling MainMenu every time a user has completed an action, the method should instead return, and MainMenu should contain a loop.

Answer 2

Encapsulation

In your code, you are excessively abusing static member functions. I see why you are doing it. You have a circular dependency between BankInterface and StaffInterface.

In order to break that dependency, you have to redesign your classes. I suggest using two main classes AccountHandler and Display.

class AccountHandler
{
public:

  void create();
  void erase();
  auto login();

  void makeWithdrawal();
  void makeDeposit();
  void showBalance();

  auto getAccountType(std::string uc);

  const auto& activeAccount() { return m_account; }
  const auto& accounts() { return m_accounts; }

private:
  Account m_account;
  std::vector<Account> m_accounts;
};

class Display
{
public:

  Display(AccountHandler& acc_handler);

  void loginMenu();
  void mainMenu();
  void showCustomerBalance();
  void staffMenu();
private:

  AccountHandler& m_acc_handler;

};

AccountHandler is responsible for creating, deleting, etc. of accounts. Display is responsible for input & output and has a dependency to AccountHandler.

enum class

Why you should use enum classes: https://stackoverflow.com/a/18335862/9226753

class vs. struct

If you only have public member variables, consider using struct vs. class. You should use classes if your class has to hold invariants. Following:

struct Account
{
  enum class Type
  {
    Customer, Staff
  };

  std::string username;
  std::string password;

  Type   type;
  double balance{0.0};
};

operator overloading

Consider implementing operator overloads as free functions. This way you set the lhs and rhs on same ground.

bool operator==(const Account& lhs, const Account& rhs)
{
  return lhs.username == rhs.username;
}

DNRY

Do not repeat yourself. You have a lot of the same code looking similar to

std::string tmp;
std::cin >> tmp; 

Instead, put that into a free function.

template<typename T>
T readFromCin()
{
  T tmp;
  std::cin >> tmp;
  return tmp;
}

It is templated so you can read whatever type you want. Use it like this:

const auto number = readFromCin<int>();
const auto word   = readFromCin<std::string>();

STL algorithms

You are using a lot of raw for loops in your code. See here why this is considered bad.

Instead, try to use STL algorithms as much as possible. See here for a nice overview of all STL algorithms. That being said, you can for example change this code

void StaffInterface::CustomerBalance() {
    std::cout << "Enter the customer's username to view their balance: \n";
    std::string cust_name;
    std::cin >> cust_name;

    for (auto &account : BankInterface::registered_accounts) {
        if (cust_name == account.username) {
            std::cout << account.balance << "\n";
            StaffMenu(); 
        }
        else {
            std::cout << "We could not detect an account with that username, please try again.\n";
            CustomerBalance();
        }
    }
}

to this

void Display::showCustomerBalance()
{
  std::cout << "Enter the customer's username to view their balance: \n";
  const auto cust_name = readFromCin<std::string>();
  const auto& accounts = m_acc_handler.accounts();
  auto it = std::find_if(accounts.begin(), accounts.end(), [&cust_name](const auto& account) {
    return cust_name == account.username;
  });

  if(it != accounts.end())
  {
    std::cout << it->balance << "\n";
    staffMenu();
  }
  else {
    std::cout << "We could not detect an account with that username, please try again.\n";
    showCustomerBalance();
  }
}

The usage of std::find_if might seem unnecessary here, but it makes the much more readable for other people, because every good C++ developer should know what each of the STL algorithms do.

const correctness

In your code you barely use const. Read here why this is considered good practice.

See adapted and working code here