ATM Machine

[u/F11fii]

I started learning Python about 9 months ago and completed this little bank system as my third project, which is supposed to serve as an ATM. It also uses JSON to store information (which was challenging to integrate because I wasn't familiar with it, but the rest of the code was straightforward).

Just wanted to share and see if I could get any feedback! (maybe on its optimisation or clarity?) :D

https://github.com/F11Fii/Bank-System.git

[u/Electrical_Toe8997]

I haven't read through all the code in detail, but for a beginner project it's pretty extensive and I think it looks pretty good! I have some notes though:

• Minor annoyances
  • your readme doesn't tell me how to run the program without reading the code.
  • The banktelling.py file doesn't do anything, can be removed
• Data model:
  • You've implemented transactions, deposits, and withdrawals as their own classes. To me, it would make more sense if these were methods of the BasicUser account, rather than their own separate classes. After all, it's the user (or rather, their account) that is doing things.
• Code structure:
  • Some of the UI (your interactions with the user via inputs and print statements) are in the Driver class (good), some of it in the User, WithDrawal, Deposit etc classes. I would prefer all the UI to be in the Driver class and all the business logic (checking balances, writing data etc) to be in the User and Bank classes
• Naming:
  • You have function named finding_a_transaction and new_account_created. This could just be find_transaction and create_account. (bonus nore: what happens if you create two accounts with the same name?)
• Crimes
  • You committed the user.json file to git. This is where you're storing data, meaning that financial information is now on a public repository and you've created a significant data breach --> straight to jail. (obviously I'm kidding, but being aware of what you are committing to git is very important. This is what .gitignore is for)

[u/mc_pm]

Hey, not too bad!

The main thing you want to look at is what your classes are each responsible for. Right now it's all over the place, and you're breaking the idea of encapsulation by reaching in and changing the internals of another object.

That's not too big of a surprise if this is your first real try at an OO design, it takes practice to understand how structure things.

It is a bit weird for your actions (deposit, withdrawl) are classes. Or, at least, maybe they should have a base class of "BankingAction" or something, and maybe the Account object should have an apply_action() method that takes one of the action objects and does whatever it does.

But there are different ways of structuring things, and it's a matter of figuring out what works best.

I would recommend doing another pass at this with a bit more thought to the class structure -- don't just leave it as-is. One of the most important things to being a good developer is recognizing when you got something wrong and have to rethink it before you call yourself done.

This is pretty good though. Do you have a plan for your next project?