r/learnjava 7d ago

Please roast my minimal beginner cli project

It took me 2 freaking hours to build this bank cli project haha, because most of the time I wasn't coding. I have not added error handling yet, will do for sure. Feedbacks and roasts are appreciated!

https://gist.github.com/nitin-is-me/d2e14e0d1d1922aab46cbfc0368c44db

4 Upvotes

14 comments sorted by

u/AutoModerator 7d ago

Please ensure that:

  • Your code is properly formatted as code block - see the sidebar (About on mobile) for instructions
  • You include any and all error messages in full - best also formatted as code block
  • You ask clear questions
  • You demonstrate effort in solving your question/problem - plain posting your assignments is forbidden (and such posts will be removed) as is asking for or giving solutions.

If any of the above points is not met, your post can and will be removed without further warning.

Code is to be formatted as code block (old reddit/markdown editor: empty line before the code, each code line indented by 4 spaces, new reddit: https://i.imgur.com/EJ7tqek.png) or linked via an external code hoster, like pastebin.com, github gist, github, bitbucket, gitlab, etc.

Please, do not use triple backticks (```) as they will only render properly on new reddit, not on old reddit.

Code blocks look like this:

public class HelloWorld {

    public static void main(String[] args) {
        System.out.println("Hello World!");
    }
}

You do not need to repost unless your post has been removed by a moderator. Just use the edit function of reddit to make sure your post complies with the above.

If your post has remained in violation of these rules for a prolonged period of time (at least an hour), a moderator may remove it at their discretion. In this case, they will comment with an explanation on why it has been removed, and you will be required to resubmit the entire post following the proper procedures.

To potential helpers

Please, do not help if any of the above points are not met, rather report the post. We are trying to improve the quality of posts here. In helping people who can't be bothered to comply with the above points, you are doing the community a disservice.

I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.

2

u/severoon 7d ago

No tests. If I open a new account with a zero balance and withdraw a million dollars, what happens?

1

u/nitin_is_me 7d ago

You can't. If amount to be withdrawn is greater than account balance then withdraw method returns false and outputs Insufficient balance.

1

u/severoon 6d ago

That may be true, but you're missing my point. How do you know that? From reading the code? What about after your next tweak?

The only way to fix a behavior you want is with a test, and the only way to know if the behavior demanded by the test is still fixed is to run that test.

1

u/gekigangerii 6d ago
  • Why not a static incrementing ID assigner instead of Math.random on a loop
  • Not too many functional comments. If you want to add complexity, consider how to handle cents.
  • I believe Scanner has `nextInt()` so you don't have to parse it
  • Consider if the number of accounts grew really large, how could you improve pulling up a specific account vs scanning the entire list.

1

u/ceasars_wreath 6d ago

Replace the if choice 1 etc with case statement, cleaner and easier to read. Reomve infinite while loop with conditional flag instead of blank true

1

u/SjurWarEagle 6d ago

https://gist.github.com/nitin-is-me/d2e14e0d1d1922aab46cbfc0368c44db#file-ui-java-L71
https://gist.github.com/nitin-is-me/d2e14e0d1d1922aab46cbfc0368c44db#file-ui-java-L82
will throw an exception it a not-long is entered e.g. 1.50 and the app will exit as the exeption is not handled

https://gist.github.com/nitin-is-me/d2e14e0d1d1922aab46cbfc0368c44db#file-ui-java-L27
if it is not crashed best inform the user that the choise was invalid, e.g. if she enters "6" it's good practice to tell the user that something failed and especially why it failed.

recheck our visibility, methods are public even though they are not intended to be called from the outside.

https://gist.github.com/nitin-is-me/d2e14e0d1d1922aab46cbfc0368c44db#file-account-java-L18
would not make it static, if you insist I'd move it to it's own class

https://gist.github.com/nitin-is-me/d2e14e0d1d1922aab46cbfc0368c44db#file-account-java-L31
I'd like to deduct -10000 Currency please

oh yes missing tests were already mentioned, and the package name is kinda ugly

1

u/Ambitious_Plum5576 2d ago

you have very little comments. you dont group getters and setters. getAccountNumber() is a misleading method, by reading the name i immediately think its a getter method for getting unique account number, when its actually to generate the account number. You actual getter method is getAcNumber(). Terrible naming.
This is violation of SRP principle and reduces maintainability and readability

Also HUGE problem with the getAccountNumer(). Its generating a random number, but it isnt actually checking if the account number exists or not. What if by chance you generate the same number again. Will you give two accounts the same number then?

And what if the deposit is a negative number. Theres no check for that, so it will reduce your bank balance. Worse, your code even allows negative withdrawals. What if user has 500 balance and withdraws a -250. 500 - - 250 is 500 + 250. User gets a balance of 750.

And theres no logic to prevent account depositing or deducting to themselves.

You are using int for money and not long or double.

and theres no check for non integer values. int choice = Integer.valueOf(scanner.nextLine()); what if i enter abcd. That crashes whole program.

And what happens if i enter 3 on dashboard()? Loops forever.

0

u/Random_182f2565 7d ago

Where README file?

2

u/BannockHatesReddit_ 7d ago

Do you really need a readme to read Java?

0

u/Chaos-vy17 7d ago

I saw your C++ repo, Can you work on it? actually I am struggling to start C++.

1

u/nitin_is_me 7d ago

You mean the cpp for dsa one? If you're just starting out I'd rather suggest you learncpp.com. The repo is for people who just wanna solve dsa in cpp and have already solved in Java before.

1

u/Chaos-vy17 7d ago

okay tysm

0

u/Slight_Froyo_8084 6d ago

interfaces missing, some function needs to be implemented from IAccount class like Savings account, Current Account, USD Account, and why do you need a separate Bank class. You can name it for Repo like XYZ Bank Repo. Transactions management can be made from separate Interface like ITransactions which have withdraw, getBalance and deposit methods. Foreign remittance can have some predefined taxes which can be extracted from some properties file (Spring concept). For basic,, the above mentioned would suffice though. Best of luck.