Guys review my project

[marsh sentinel]

Its not the best but its good enough at least for me. Tell me what I can improve on this.
https://github.com/modwodmm/MiniBank
Additional features will be added like memory of last use and transaction history later as I learned reading and writing files today other than that what could be made better in the current code.

[eternal fable]

<@&987246883653156906> please have a look, thanks.

[eternal oracle]

One user can have several currency accounts and different currencies.

There may also be a bank deposit at % per annum

[nocturne pollen]

to compare strings you don't use ==

    public boolean checkPin(String enteredPin) {
        return pin == enteredPin;
    }

so instead of this

    public boolean checkPin(String enteredPin) {
        return pin.equals(enteredPin);
    }

[candid bronze]

instead of having project as a folder in the repo, move the maven project in the root directly

[marsh sentinel]

oh yeah I changed pin from int to string and forgot to change it

[candid bronze]

are you trying to add a local database system?

[marsh sentinel]

not now really but kinda yeah I will be improving it with whatever I learn new which can be added

[full frigate]

private ArrayList<Account> accounts = new ArrayList<>();

we have talked about this, in java it is usual to only use the interface List
even if you do not need a indirection and never want to use a other implementation, the pattern people want to see is

private List<Account> accounts = new ArrayList<>();

the theorectical advantage of it is that the interface List defines a contract of which operations are supported by this object but how it is implemented does not matter, so you can change the implementation at any time just at this one place and do not have to change anything else, this is called a indirection, even if you do not need it in this case, there is no downside in always using it.

e.g. you could change it to:

private List<Account> accounts = new LinkedList<>();

and everything else stays the same. but do not do that ArrayList is the best, LinkedList has only rare use cases, in 99% of cases you will only use ArrayList. E.g. in LRU caches LinkedLists are used.

[marsh sentinel]

oh yeah mb I forgot about that 😭

[candid bronze]

i do see many other changes in the project

[marsh sentinel]

me too I gonna add transaction history, showing accounts, session saving and even a ui

[candid bronze]

for ui, you may use jline

[marsh sentinel]

yeah but is kinda for the future cuz ui is a big this

[full frigate]

choice = scanner.nextInt();

never use nextInt on a Scanner on System.in

[eternal fable]

Mixing any nextXXX method with nextLine from the Scanner class for user input, will not ask you for input again but instead result in an empty line read by nextLine.

To prevent this, when reading user input, always only use nextLine. If you need an int, do

int value = Integer.parseInt(scanner.nextLine());

instead of using nextInt.

Assume the following:

Scanner scanner = new Scanner(System.in);

System.out.println("Enter your age:");
int age = scanner.nextInt();
System.out.println("Enter your name:");
String name = scanner.nextLine();

System.out.println("Hello " + name + ", you are " + age + " years old");

When executing this code, you will be asked to enter an age, suppose you enter 20.
However, the code will not ask you to actually input a name and the output will be:

Hello , you are 20 years old.

The reason why is that when you hit the enter button, your actual input is

20\n

and not just 20. A call to nextInt will now consume the 20 and leave the newline symbol \n in the internal input buffer of System.in. The call to nextLine will now not lead to a new input, since there is still unread input left in System.in. So it will read the \n, leading to an empty input.

So every user input is not only a number, but a full line. As such, it makes much more sense to also use nextLine(), even if reading just an age. The corrected code which works as intended is:

Scanner scanner = new Scanner(System.in);

System.out.println("Enter your age:");
// Now nextLine, not nextInt anymore
int age = Integer.parseInt(scanner.nextLine());
System.out.println("Enter your name:");
String name = scanner.nextLine();

System.out.println("Hello " + name + ", you are " + age + " years old");

The nextXXX methods, such as nextInt can be useful when reading multi-input from a single line. For example when you enter 20 John in a single line.

spectre_1_0 • used /tag id: scanner · May 19, 2026, 1:43 PM

[marsh sentinel]

oh yeah nice find

[full frigate]

to get rid of the switch, which is good when you want to add more options you can use a HashMap:

public class Main {
    private static Map<Integer, Runnable> actions = new HashMap<>();
    private static Bank bank = new Bank();

    static {
        actions.put(1, Main::deposit);
        actions.put(2, Main::withdraw);
    }

    private static void deposit() {
        // ...
    }
    
    private static void withdaw() {
        // ...
    }

    // etc.

    public static void main(String[] args) {
        int choice = 0;
        // your logic to get the choice from the user
        actions.getOrDefault(choice, () -> System.out.println("invalid choice!")).run();
    }
}
    

or use Function<Bank, Void> if you want to instanciate the bank in the main method instead of making it a static member

this is usually much neater than a giant switch or a huge if-else if-else cascade

[marsh sentinel]

I'm really close to learning it and as soon as I do I'll def use it I'm really not good at java rn

[full frigate]

Maps are life

i always use maps

you then also might want to store the accounts in a Map<String, Account>, so that you can search in amortised O(1) based on the username.

[full frigate]

but really look for this error, you made it all over the place in your project. when you have a Scanner on System.in, only use nextLine

[candid bronze]

@marsh sentinel why do you have so many tags?

u have made 9 tags for just 4 months of development

at least 3-2 tags are ok

but 9 is too much

[marsh sentinel]

lol idk for no reason

[full frigate]

if (depositAmount < 0 || depositAmount == 0) 

you can use this instead:

if (depositAmount <= 0)

instead of

if (accounts.size() == 0)

you can do

if (accounts.isEmpty())

                        while(!correctWithdrawal) {
                            try {
                                System.out.println("Enter the amount to withdraw:");
                                withdrawalAmount = scanner.nextDouble();
                                correctWithdrawal = true;
                            }
                            catch(InputMismatchException e){
                                System.out.println("Please enter an actual amount to withdraw.");
                                scanner.nextLine();
                            }

do not read new input in the catch block

do:

while (!correctWithdrawal) {
    try {
        System.out.println("Enter the amount to withdraw:");
        withdrawalAmount = Double.parseDouble(scanner.nextLine());
        correctWithdrawal = true;
    }
    catch (NumberFormatException e) {
        System.out.println("Enter a correct number");
    }
}

[marsh sentinel]

oh ok I didn't know you can do accounts.isEmpty() noice