5
\$\begingroup\$

This is my 2nd C++ project, so I created a bank management system. This was particularly hard because I had to understand OOP. I still don't fully get it, but I'm sure I'll improve as I do more projects. Please feel free to give me feedback on what I could do to improve the project as well as tips on better practices.

Here is the code:

#include <iostream>
#include <iomanip>
#include <ctime>
using namespace std;

class BankAccount {
  private:
    double balance;
  public:
    string accountName;
    // Constructor to initialize balance
    BankAccount(double initial_balance) {
      if(initial_balance >=0) {
        balance = initial_balance;
      } else {
        balance = 0;
        cout << "Initial balance cannot be negative. Setting balance to 0." << endl;
      }
    }

    // Method to deposit money
    void deposit(double amount) {
      if (amount > 0) {
        balance += amount;
        cout << "Deposit $" << amount << ". Your Balance is $" << balance << "." << endl;
      } else {
        cout << "Deposit amount must be positive." << endl;
      }
    }

    // Method to withdraw money
    void withdraw(double amount) {
      if (amount > 0) {
        if (amount <= balance) {
          balance -= amount;
          cout << "Withdraw $" << amount << ". Your Balance is $" << balance << "." << endl;
        } else {
          cout << "Insuffient balance. Failed to withdraw." << endl;
        }
      } else {
        cout << "Withdraw amount must be positive." << endl;
      }
    }

    // Method to check balance
    double getBalance() const {
      return balance;
    }

};

int main() {
  srand(time(NULL));
  int account_No = rand() % 10000000000 + 1000000000; // Generates some random numbers
  int account_Options;
  double amount;
  BankAccount user1(0);
  // Welcome message and Input for Name
  cout << "Welcome to Simple Bank \n"
          "What is your first name? \n";
  cin >> user1.accountName;
  // Account Details section
  cout << "\nHello " << user1.accountName << ", these are your Bank details: \n";
  cout << "\nAccount Name: " << user1.accountName << endl;
  cout << "Account Number: " << account_No << endl;
  // Account Applicable Options

  while (true) 
  { 
    cout << "How would you like to procceed?\n" << endl;
      cout << "1.Deposit\n"
              "2.Withdraw\n"
              "3.Check Balance\n"
              "4.Exit\n";
      cin >> account_Options;

      const int option1= 1, option2 = 2, option3 = 3, option4 = 4;
      switch (account_Options) {
        case option1:
          int deposit;
          cout << "How much would you like to deposit" << endl;
          cin >> amount;
          user1.deposit(amount);
           break;
        case option2:
          int withdraw;
          cout << "How much would you like to withdraw? \n";
          cin >> amount;
          user1.withdraw(amount);
          break;
        case option3:
          cout << "Your current balance is: $" << user1.getBalance() << endl;
          break;
        case option4:
          cout << "Thank you for using Simple Bank!" << endl;
          return 0;
          break;
        default:
          cout << "Invalid Option" << endl;
          break;
  };
  
      }
      return 0;
}
\$\endgroup\$
0

1 Answer 1

5
\$\begingroup\$

Do not use using namespace std:

I have to emulate namespaces in C by prefixing each function name with libname_, which looks unaesthetic, is prone to conflicts, and makes me type more. C++ offers namespaces, which is a feature, not a nuisance. Treasure what good you have been offered. If you do not want to type std:: before cout and cin, consider doing:

using std::cout; 
using std::cin;

instead of importing all the symbols.

See also: What's the problem with "using namespace std;"?.

Avoid srand() and rand() in C++ code:

C++11 introduced new random number facilities that are in many ways superior to the old rand() based stuff. See: How to generate a random number in C++?.

An int might only have 16 bits:

int account_No = rand() % 10000000000 + 1000000000; // Generates some random numbers

An int is only required to hold values in the range −32768 to 32767. So the above operation can overflow, which would result in undefined behavior. Use a larger type perhaps?

Simplify main():

You can move the calls to std::cout for the welcoming part to a separate function, and the part that is printing all the bank details to a separate function, and the part that is printing all the options to a separate function.

The main infinite loop can be moved to a separate function too. This would make your code modular, more readable, and separate different responsibilities.

return 0 is the default behavior:

The absence of a return statement in main() is equivalent to return 0, which is equivalent to return EXIT_SUCCESS, so you can omit it.

But even if you're going to keep it, consider using EXIT_SUCCESS and EXIT_FAILURE instead of magic values. Too often I see people exiting on failure with 0 instead of EXIT_FAILURE.

Hiding the switch's case values does not help readability:

const int option1= 1, option2 = 2, option3 = 3, option4 = 4;
      switch (account_Options) {
        case option1:
          ...
        case option2:
          ...
        case option3:
          ...
        case option4:
          ...
  };

Omit option1-option4 and write 1-4 directly.

Do not flush the output buffer unnecessarily:

cout << "How would you like to procceed?\n" << endl;

std::endl flushes the underlying buffer along with printing a newline, which is neither needed, nor appear to be intended. Replace it with a simple '\n'.

Do not ignore input errors:

cin >> amount;

This can fail. The least you could do is:

std::cin >> amount;

if (!std::cin) {
    std::cerr << "error: invalid input.\n";
    return EXIT_FAILURE;
}

Use an automatic code-formatter:

You seem to have indented the code manually, which shows from the unaligned braces and indents. Consider using one of clang-format, GNU Indent, Astyle et cetera. It would save your time too.

Error messages go to standard error:

In the case that standard output was redirected to a file or a pipe, the error message would go unseen.

cout << "Deposit amount must be positive." << endl;

should be:

std::cerr << "Deposit amount must be positive.\n";

Same for other error messages.

\$\endgroup\$
3
  • \$\begingroup\$ “The absence of a return statement in main() is equivalent to return EXIT_SUCCESS, which is equivalent to exit(EXIT_SUCCESS) …” No, it is absolutely not equivalent. exit() / std::exit() is a C function; returning x from main() is equivalent to exit(x) in C… not C++. Never use exit() / std::exit() in C++. (Also, the absence of a return statement in main() is not equivalent to return EXIT_SUCCESS;, but rather to return 0;. EXIT_SUCCESS is not necessarily 0.) \$\endgroup\$
    – indi
    Commented Jun 14 at 22:53
  • \$\begingroup\$ Ah yes, I forgot. I did not claim that the value of EXIT_SUCCESS is equivalent to 0, but that return 0 is equivalent to return EXIT_SUCCESS and that 0 and EXIT_SUCCESS both indicate success. "If the value of status is zero or EXIT_SUCCESS, an implementation-defined form of the status successful termination is returned." \$\endgroup\$
    – Harith
    Commented Jun 14 at 23:30
  • 1
    \$\begingroup\$ You can teach your text editor/IDE to write stuff like std:: on a keypress, in fact a good one will suggest code options and then something like std::cout<< is just pressing tab (for example). You could also use an enum for your switch statement if you want it to be more descriptive. \$\endgroup\$
    – Lozminda
    Commented Jun 20 at 1:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.