Diary applications with accounts

I didn't write the login code yet.

Main

```
static AccountLogin objAccountLogin = new AccountLogin();
static AccountRemover objAccountRemover = new AccountRemover();
static AccountCreator objAccountCreator = new AccountCreator();
static Accounts objAccounts = new Accounts();

public static void main(String[] args) {

startDiary();

}

public static void mainMenu(){
System.out.println("");
System.out.println("Welcome to Diary!");
System.out.println("1- New Account");
System.out.println("2- Login To Your Account");
System.out.println("3- Remove Account");
System.out.println("4- Exit");
System.out.println("");
usersChoice();
}

public static void usersChoice(){
int choice = 0;
while(true){
try{
Scanner scan = new Scanner(System.in);
System.out.print("Choose an option: ");
choice = scan.nextInt();
break;
}
catch(Exception e){
System.out.println("Invalid Input!\n");
}
}

switch (choice){
case 1:
choiceNewAccount();
case 2:
choiceLogin();
case 3:
choiceRemoveAccount();
case 4:
exit();
break;
default:
System.out.println("Invalid Input!\n");
usersChoice();
}

}

public static void startDiary(){
objAccounts.loadAccounts();
mainMenu();
}

public static void choiceNewAccount(){
objAccountCreator.createAccount();
mainMenu();
}

public static void choiceLogin(){
objAccountLogin.login();
mainMenu();
}

public static void choiceRemoveAccount(){
objAccountRemover.removeAccount();
mainMenu();

}

public static void exit(){
while(true){
try{
Scanner exit = new Scanner(System.in);
System.out.print("Are you sure you want to exit? (y/n): ");
char choice = exit.next().charAt(0);

Let's start at the beginning...

usersChoice() tends to call itself again and again and again, leaving stuff on the stack. I would suggest putting the whole choice thing inside a...

while(true) {

... do your stuff here

}

...as the System.exit() in exit() will end the program anyway. Also you are not closing your scanner. In one piece of code you used the try-catch-with-resources...

try (Scanner usernamesScanner = new Scanner(usernamesFile)) {

...stick with that.

You are creating at least 3 scanners, might be a good idea to see if you can move that "read something from file or console" code into its own method.

loadUsernames() and loadPasswords() are the same code, and they only differ in where the text is stored, so you can merge that into one method. Same with the addToUsernames() and addToPasswords() - you even forget to change the error message when copy and pasting. Copy and paste, especially your own code, is not a good idea: If you need to change the code, you will very likely need to remember two places to change it. Merge them into one method by identifying which parts change.

You are using a method canLogin(), but never declare it. Also, the name implies that the answer is true or false, but it seems to return the loginName. Method names should intuitively make clear what they will do, this one absolutely doesn't.

Your methods tend to be a bit long. For example, the exit-method effectively does three things:

• Read from console

• Check if the text is something

• Exit, if yes

You could write that clearer by removing at least one of this things into its own method, for example...

public void tryExit() {
    String userConfirmedExit = askFromConsole("Are you sure you want to exit? (y/n)"); // implement that method
    if ("y".equals(userConfirmedExit)) {
       System.exit(0);
    } else if (!"n".equals(userConfirmedExit)) {
       System.out.println("Invalid character!")
    } 
    // in all other cases, simply return and the loop (see above) will bring back the menu
}

The whole code is still very static-based. I would try to remove every static method besides the main one.

StackExchange Code Review Q#101086, answer score: 5