patternjavaMinor
ATM (Automated Teller Machine) Terminal Application
Viewed 0 times
terminalapplicationautomatedtellermachineatm
Problem
I consider myself a beginner in Java. Therefore I want to improve and learn about Java(8) and OOP as much as I can. Feedback about:
is very much appreciated.
If you could review with regard to these questions, then I'd be very grateful:
This is "Automated Teller Machine Terminal Application" in Java:
You can do typical transaction like deposit, withdraw, and show balance. It persistently saves your transactions in an external file so you can see your changes after restarting the application.
It also (tries to) handle all exception cases.
GitHub
Main file (not much in here)
In the model folder I got my AccountCard file:
```
package model;
import model.exceptions.DepositNegativeBankTransfer;
import model.exceptions.ExceedLimitTransfer;
import model.exceptions.OverdrawBankTransfer;
import model.exceptions.ZeroBankTransfer;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.Map;
public class AccountCard {
private String pin = null;
private int amount = 0;
DateFormat dateFormat = new SimpleDateFormat("yyyy/MM/dd HH");
private Map transferHistory = new DefaultLinkedHashMap<>(0);
public AccountCard(String pin, int amount, String withdrawDate, int timesWithDraw) {
this.pin = pin;
this.amount = amount;
this.transferHistory.put(withdrawDate, timesWithDraw);
}
public void setPin(String pin) {
this.pin = pin;
}
public String getPin() {return this.pin;}
public boolean isPinCorrect(String pinInputByUser) {
return this.pin.equals(pinInputByUser);
}
publ
- performance
- clearn code
is very much appreciated.
If you could review with regard to these questions, then I'd be very grateful:
- Is having multiple returns in the function
pinIsValidFora bad pattern? How would you write it differently?
- Are the classes too long (they nearly got 300 lines of code)? What would be a good size for a Java-class
This is "Automated Teller Machine Terminal Application" in Java:
You can do typical transaction like deposit, withdraw, and show balance. It persistently saves your transactions in an external file so you can see your changes after restarting the application.
It also (tries to) handle all exception cases.
GitHub
Main file (not much in here)
import controller.Atm;
public class Main {
public static void main(String[] args) {
Atm.start();
}
}In the model folder I got my AccountCard file:
```
package model;
import model.exceptions.DepositNegativeBankTransfer;
import model.exceptions.ExceedLimitTransfer;
import model.exceptions.OverdrawBankTransfer;
import model.exceptions.ZeroBankTransfer;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.Map;
public class AccountCard {
private String pin = null;
private int amount = 0;
DateFormat dateFormat = new SimpleDateFormat("yyyy/MM/dd HH");
private Map transferHistory = new DefaultLinkedHashMap<>(0);
public AccountCard(String pin, int amount, String withdrawDate, int timesWithDraw) {
this.pin = pin;
this.amount = amount;
this.transferHistory.put(withdrawDate, timesWithDraw);
}
public void setPin(String pin) {
this.pin = pin;
}
public String getPin() {return this.pin;}
public boolean isPinCorrect(String pinInputByUser) {
return this.pin.equals(pinInputByUser);
}
publ
Solution
Thanks for sharing the code.
Here is what I think about it:
Naming
Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.
On the bright side you follow the Java Naming Conventions.
But in Java there is an exception for boolean variables and methods: they should start with
Entry class.
In general it is a good thing to have a tiny entry class. We usually use it to switch from static context to object context.
Your entry class simply delegates to another static method in a different class. Therefore it is almost useless.
OOP
Doing OOP means that you follow certain principles which are (amongst others):
Interfaces, abstract classes, or inheritance support that principles and should be used as needed.
In that sense your code really does implement some of the OO principles.
But your entire program is in static context with is bad for a couple of reasons (especially for a "training" program):
If you would actually use objects instead of just
The typical example is the switch in your method
the OOP approach to this is to define a common interface for the operations.
The we have a separate class for each operation (mot likely in its own class)
Some of that operations need resources from the ATM:
There are different ways to access this implementations. The easiest way is to put them in a Map when you initialize the ATM:
then your method
benefits are:
I assume the AtmConsole is the console of the Atm where all the transactional information and main menu is displayed. How would the AtmConsole look like?
Im my world there could be different console types. Therefore I had an AtmConsole interface:
And This would be my implementation:
```
class AtmConsoleCommandLine implements AtmConsole {
private final Scanner input;
public AtmConsole(Scanner input){
th
Here is what I think about it:
Naming
Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.
On the bright side you follow the Java Naming Conventions.
But in Java there is an exception for boolean variables and methods: they should start with
is or has like instead of atmIsActivated it should be isAtmActivated.Entry class.
In general it is a good thing to have a tiny entry class. We usually use it to switch from static context to object context.
Your entry class simply delegates to another static method in a different class. Therefore it is almost useless.
OOP
Doing OOP means that you follow certain principles which are (amongst others):
- information hiding / encapsulation
- single responsibility
- separation of concerns
- KISS (Keep it simple (and) stupid.)
- DRY (Don't repeat yourself.)
- "Tell! Don't ask."
- Law of demeter ("Don't talk to strangers!")
- favor polymorphism over branching
Interfaces, abstract classes, or inheritance support that principles and should be used as needed.
In that sense your code really does implement some of the OO principles.
But your entire program is in static context with is bad for a couple of reasons (especially for a "training" program):
- no polymorphism
- no dependency injection
- hard to UnitTest
If you would actually use objects instead of just
static methods you could make that code simpler and easier to extend. The typical example is the switch in your method
processUserSelection:the OOP approach to this is to define a common interface for the operations.
interface AtmOperation {
void operateOn(AccountCard account);
}The we have a separate class for each operation (mot likely in its own class)
class WithdrawOperation implements AtmOperation {
@Override
void operateOn(AccountCard account){
}
}
class DepositOperation implements AtmOperation {
@Override
void operateOn(AccountCard account){
}
}
class BalanceOperation implements AtmOperation {
@Override
void operateOn(AccountCard account){
}
}Some of that operations need resources from the ATM:
class BalanceOperation implements AtmOperation {
/* pretend we have a class AtmConsole with methods .redraw() and .println() */
private final AtmConsole console;
public BalanceOperation( AtmConsole console){
this.console = console;
}
@Override
void operateOn(AccountCard account){
console.redraw();
Locale currentLocale = Locale.US;
int amount = account.getBalance();
NumberFormat currencyFormatter = NumberFormat.getCurrencyInstance(currentLocale);
console.println("Your current balance is now " + currencyFormatter.format(amount));
}
}
class InputErrorOperation implements AtmOperation {
private final AtmConsole console;
public BalanceOperation( AtmConsole console){
this.console = console;
}
@Override
void operateOn(AccountCard account){
console.redraw();
console.println("Unknown operation please try again");
}
}There are different ways to access this implementations. The easiest way is to put them in a Map when you initialize the ATM:
// ...
private final Map operations = new HashMap<>();
private final AtmConsole console = new AtmConsole();
private final AtmOperation invalidInputOperation = new InvalidInputOperation(console);
public static void start() {
operations.put("w",new WithdrawOperation(console));
operations.put("d",new DepositOperation(console));
operations.put("s",new BalanceOperation(console));
operations.put("e",new ReturnCardOperation(console));
atmIsActivated = true;
// ...then your method
processUserSelection would change to:private static void processUserSelection(AccountCard account) {
String userInput = Helper.getUserInput();
AtmOperation operation = operations.getOrDefault(userInput.toLowerCase(),invalidInputOperation);
operation.operateOn(account);
}benefits are:
- configurations, definition and usage of the operations are clearly separated.
- you can change the operations without changing the place(s) where they are used.
- you can put each operation in its own class file
- you can easily write UnitTests for each operation to verify and document their behavior.
I assume the AtmConsole is the console of the Atm where all the transactional information and main menu is displayed. How would the AtmConsole look like?
Im my world there could be different console types. Therefore I had an AtmConsole interface:
interface AtmConsole{
void redraw();
void println(String message);
/* most likely it should also handle the input
Because it is the User Interface: */
String aqireInput();
}And This would be my implementation:
```
class AtmConsoleCommandLine implements AtmConsole {
private final Scanner input;
public AtmConsole(Scanner input){
th
Code Snippets
interface AtmOperation {
void operateOn(AccountCard account);
}class WithdrawOperation implements AtmOperation {
@Override
void operateOn(AccountCard account){
}
}
class DepositOperation implements AtmOperation {
@Override
void operateOn(AccountCard account){
}
}
class BalanceOperation implements AtmOperation {
@Override
void operateOn(AccountCard account){
}
}class BalanceOperation implements AtmOperation {
/* pretend we have a class AtmConsole with methods .redraw() and .println() */
private final AtmConsole console;
public BalanceOperation( AtmConsole console){
this.console = console;
}
@Override
void operateOn(AccountCard account){
console.redraw();
Locale currentLocale = Locale.US;
int amount = account.getBalance();
NumberFormat currencyFormatter = NumberFormat.getCurrencyInstance(currentLocale);
console.println("Your current balance is now " + currencyFormatter.format(amount));
}
}
class InputErrorOperation implements AtmOperation {
private final AtmConsole console;
public BalanceOperation( AtmConsole console){
this.console = console;
}
@Override
void operateOn(AccountCard account){
console.redraw();
console.println("Unknown operation please try again");
}
}// ...
private final Map<String,AtmOperation> operations = new HashMap<>();
private final AtmConsole console = new AtmConsole();
private final AtmOperation invalidInputOperation = new InvalidInputOperation(console);
public static void start() {
operations.put("w",new WithdrawOperation(console));
operations.put("d",new DepositOperation(console));
operations.put("s",new BalanceOperation(console));
operations.put("e",new ReturnCardOperation(console));
atmIsActivated = true;
// ...private static void processUserSelection(AccountCard account) {
String userInput = Helper.getUserInput();
AtmOperation operation = operations.getOrDefault(userInput.toLowerCase(),invalidInputOperation);
operation.operateOn(account);
}Context
StackExchange Code Review Q#162667, answer score: 3
Revisions (0)
No revisions yet.