Storing passwords
The recommended approach for reading passwords is to use a char[] array, instead of a String. The main reason for this, as explained in this SO answer, is that immutable Strings leave your passwords accessible until garbage collection (GC) kicks in, with a malicious process performing a memory dump of your Java process to do so.
Helpfully, Console.readPassword() returns a char[] array while hiding the user input on the console:
char[] password = System.console().readPassword();
// validate password
// replace the char[] array contents immediately after validation
Arrays.fill(password, ' ');
Reading data, and data modeling
Currently, you are hardcoding the account details as mere space-delimited Strings. This is not desirable from a data modeling perspective as it is non-trivial to:
- 'Upgrade' the formatting to support newer or different fields,
- Make edits, and
- Testing.
These 'pain points' are evident in the way you are doing validation currently:
if (acctNum.equals(a.substring(0, a.indexOf(" "))) &&
pwd.equals(a.substring(a.indexOf(" ")+1,a.lastIndexOf(" "))))
// BTW looks like you have a typo/bug here
// return result = a.substring(a.lastIndexOf(" ") + 1);
result = a.substring(a.lastIndexOf(" ") + 1);
- You have to manually identify where each column starts and ends.
- The code assumes the account number is in the first field, the password is the second, and the result is in third and final field.
- (related to the earlier section, and a big security no-no) Your passwords are in clear text, instead of being encrypted (or at least hashed).
- You have hardcorded three accounts into three different variables, which is not scalable.
So how can this be done differently?
Let's start with a class called Account:
public class Account {
String name;
String encryptedPassword;
double balance;
public class Account(String name, String encryptedPassword, double balance) {
this.name = name;
this.encryptedPassword = encryptedPassword;
this.balance = balance;
}
// create getters and setters too
public boolean isMatching(String name, String encryptedPassword) {
return Object.equals(this.name, name)
&& Objects.equals(this.encryptedPassword, encryptedPassword);
}
}
With a class-based design, adding new fields or changing their data types is a lot easier. Now, we need a mechanism that knows how to interact with multiple accounts, and perform validation on picking an account. An AccountManager sounds like one for the job:
public class AccountManager {
// using a List here for simplicity
private final List<Account> accounts = getAccounts();
// no modifiers used as an illustration to show how testing can override this
private List<Account> getAccounts() {
// read from a source, e.g. database, and return as a List of Account objects
}
public Account getAccount(String accountName, char[] password) {
String encryptedInput = encryptPassword(password);
Arrays.fill(password, ' ');
// simple implementation that loops through the accounts
for (Account account : accounts) {
if (account.isMatching(accountName, encryptedInput)) {
return account;
}
}
return null;
}
}
The use of a List lets us store one, three, 10, or more accounts easily without having to deal with a multitude of variables. We also need to encrypt the password first, then rely on the Account.isMatching() method that lets us easily identify which account matches on the inputs. In this case, we no longer have to worry how to read space-delimited Strings in order to identify the account name and password parts correctly.
Variable names
public static double deposit(double x, double y)
{
double depositAmt = y, currentBal = x;
// ...
}
public static double withdraw(double x, double y)
{
double withdrawAmt = y, currentBal = x, newBalance;
// ...
}
I thought it is... mildly amusing that you know what the appropriate variable names should used as the method parameters, and yet you stuck with x and y only to perform the assignment afterwards. They should be rewritten as such:
public static double deposit(double currentBalance, double depositAmount)
{
// ...
}
public static double withdraw(double newBalance, double withdrawalAmount)
{
// ...
}
Reducing code duplication
You have very similar ways of printing the balances in the displayBalance(), deposit() and withdrawal() methods, differing only in the output. This ties in with the use of a suitable model class as explained in the earlier section. For example:
public class Account {
// ...
public boolean deposit(double amount) {
// ...
}
public boolean withdrawal(double amount) {
// ...
}
@Override
public String toString() {
return toString("Current");
}
public String toString(String description) {
return String.format("Your %s Balance is $%.2f", description, balance);
}
}
With these methods, it simplifies how money is deposited or withdrawn from an Account, and how the balance can be shown:
// show current balance
System.out.println(account);
// do a deposit
boolean isMoneyDeposited = account.deposit(100);
if (isMoneyDeposited) {
System.out.println(account.toString("New"));
}
try-with-resources
If you are on Java 7 and above, you should use try-with-resources for efficient handling of the underlying I/O resource:
public static void main(String[] args) {
try (Scanner scanner = new Scanner(System.in)) {
// ...
}
}
Consistent bracing style
If you are more comfortable with orphan/standalone braces on each line, then do so consistently to improve the code readability. :) Your class declaration and the main() method deviate from this, and some of your if-else statements are also missing braces.