Skip to main content
Made the review less personal and fixed some linguistic mistakes.
Source Link

Don't use floats or doubles for money/currency.

https://stackoverflow.com/questions/3730019/why-not-use-double-or-float-to-represent-currency

Naming

As Timothy pointed out, methods starting with is are expected to return a boolean value.

A more readable name for isAccountExist would be isExistingAccount.

For depositing you used depositMoney is used, but for withdrawing you used withdrawel is used. For consistency it would be better to rename thiswithdrawel to withdrawMoney.

Concurrency

YourThe code does things that have to do with money. When writing software that relates to money (or anything with transactions), it is concerned it's important to think about things like thread-safety. Java has things in it's standard library to help with this. A few examples of this are atomic variables and synchronization.

Exception handling

As you mentioned you didn't have time to implement exception handling. From a quick look at yourthe code you use two types of functions which can throw andan exception are used. These are the next and parse functions.

When sc.nextInt() is called and no input is given it will throw a NoSuchElementException. If the input can'tcannot be parsed as an integer it will throw aan InputMismatchException. To prevent this and handle these exceptions you can use aA try/catch block can be used to handle these exceptions and prevent them from crashing your application.

Instead ofWithout exception handling:

int accountID = sc.nextInt();

You should do something like thisWith exception handling:

int accountID = 0;

while (true) {
  try {
    accountID = sc.nextInt();
    break;
  } catch (NoSuchElementException e) {
    sc.nextLine();
  }
}

In this example itthe code will try to read the input and parse it as an integer. If the input is an integer itaccountID will be set the accountID and the code will break from the while loop. If a NoSuchElementException or an InputMismatchException occurs the code will go to the next line for input and try again. The reason we only have to catchOnly the NoSuchElementException ishas to be caught because InputMismatchException is a subclass of the NoSuchElementException.

The Integer.parseInt() function for example will throw a NumberFormatException if the input cannot be parsed. In a few instances you can actually avoid using the Integer.parseInt() functioncan actually be avoided by using the sc.nextInt() function like you did before, insteadin other parts of Integer.parseInt(sc.next())the code.

Instead of:

int choice = Integer.parseInt(sc.next());

You could doCould be replaced to avoid extra exception handling by:

int choice = sc.nextInt();

Don't use floats or doubles for money/currency.

https://stackoverflow.com/questions/3730019/why-not-use-double-or-float-to-represent-currency

Naming

As Timothy pointed out, methods starting with is are expected to return a boolean value.

A more readable name for isAccountExist would be isExistingAccount.

For depositing you used depositMoney, but for withdrawing you used withdrawel. For consistency it would be better to rename this to withdrawMoney.

Concurrency

Your code does things that have to do with money. When money is concerned it's important to think about things like thread-safety. Java has things in it's standard library to help with this. A few examples of this are atomic variables and synchronization.

Exception handling

As you mentioned you didn't have time to implement exception handling. From a quick look at your code you use two types of functions which can throw and exception. These are the next and parse functions.

When sc.nextInt() is called and no input is given it will throw a NoSuchElementException. If the input can't be parsed as an integer it will throw a InputMismatchException. To prevent this and handle these exceptions you can use a try/catch block.

Instead of:

int accountID = sc.nextInt();

You should do something like this:

int accountID = 0;

while (true) {
  try {
    accountID = sc.nextInt();
    break;
  } catch (NoSuchElementException e) {
    sc.nextLine();
  }
}

In this example it will try to read the input and parse it as an integer. If the input is an integer it will set the accountID and break from the while loop. If a NoSuchElementException or InputMismatchException occurs the code will go to the next line for input and try again. The reason we only have to catch the NoSuchElementException is because InputMismatchException is a subclass of the NoSuchElementException.

The Integer.parseInt() function for example will throw a NumberFormatException if the input cannot be parsed. In a few instances you can actually avoid using the Integer.parseInt() function by using sc.nextInt() like you did before, instead of Integer.parseInt(sc.next()).

Instead of:

int choice = Integer.parseInt(sc.next());

You could do:

int choice = sc.nextInt();

Don't use floats or doubles for money/currency.

https://stackoverflow.com/questions/3730019/why-not-use-double-or-float-to-represent-currency

Naming

As Timothy pointed out, methods starting with is are expected to return a boolean value.

A more readable name for isAccountExist would be isExistingAccount.

For depositing depositMoney is used, but for withdrawing withdrawel is used. For consistency it would be better to rename withdrawel to withdrawMoney.

Concurrency

The code does things that have to do with money. When writing software that relates to money (or anything with transactions), it is important to think about things like thread-safety. Java has things in it's standard library to help with this. A few examples of this are atomic variables and synchronization.

Exception handling

As you mentioned you didn't have time to implement exception handling. From a quick look at the code two types of functions which can throw an exception are used. These are the next and parse functions.

When sc.nextInt() is called and no input is given it will throw a NoSuchElementException. If the input cannot be parsed as an integer it will throw an InputMismatchException. A try/catch block can be used to handle these exceptions and prevent them from crashing your application.

Without exception handling:

int accountID = sc.nextInt();

With exception handling:

int accountID = 0;

while (true) {
  try {
    accountID = sc.nextInt();
    break;
  } catch (NoSuchElementException e) {
    sc.nextLine();
  }
}

In this example the code will try to read the input and parse it as an integer. If the input is an integer accountID will be set and the code will break from the while loop. If a NoSuchElementException or an InputMismatchException occurs the code will go to the next line for input and try again. Only the NoSuchElementException has to be caught because InputMismatchException is a subclass of NoSuchElementException.

The Integer.parseInt() function will throw a NumberFormatException if the input cannot be parsed. In a few instances Integer.parseInt() can actually be avoided by using the sc.nextInt() function like in other parts of the code.

int choice = Integer.parseInt(sc.next());

Could be replaced to avoid extra exception handling by:

int choice = sc.nextInt();
Source Link
Lau G
  • 218
  • 1
  • 6

Don't use floats or doubles for money/currency.

https://stackoverflow.com/questions/3730019/why-not-use-double-or-float-to-represent-currency

Naming

As Timothy pointed out, methods starting with is are expected to return a boolean value.

A more readable name for isAccountExist would be isExistingAccount.

For depositing you used depositMoney, but for withdrawing you used withdrawel. For consistency it would be better to rename this to withdrawMoney.

Concurrency

Your code does things that have to do with money. When money is concerned it's important to think about things like thread-safety. Java has things in it's standard library to help with this. A few examples of this are atomic variables and synchronization.

Exception handling

As you mentioned you didn't have time to implement exception handling. From a quick look at your code you use two types of functions which can throw and exception. These are the next and parse functions.

When sc.nextInt() is called and no input is given it will throw a NoSuchElementException. If the input can't be parsed as an integer it will throw a InputMismatchException. To prevent this and handle these exceptions you can use a try/catch block.

Instead of:

int accountID = sc.nextInt();

You should do something like this:

int accountID = 0;

while (true) {
  try {
    accountID = sc.nextInt();
    break;
  } catch (NoSuchElementException e) {
    sc.nextLine();
  }
}

In this example it will try to read the input and parse it as an integer. If the input is an integer it will set the accountID and break from the while loop. If a NoSuchElementException or InputMismatchException occurs the code will go to the next line for input and try again. The reason we only have to catch the NoSuchElementException is because InputMismatchException is a subclass of the NoSuchElementException.

The Integer.parseInt() function for example will throw a NumberFormatException if the input cannot be parsed. In a few instances you can actually avoid using the Integer.parseInt() function by using sc.nextInt() like you did before, instead of Integer.parseInt(sc.next()).

Instead of:

int choice = Integer.parseInt(sc.next());

You could do:

int choice = sc.nextInt();