Today I went to an interview with the Cleartrip software company. For the first round programming, we need to build a mini bank application.

In the question paper they clearly mentioned that no data persistence is required.

• Create an account


• Deposit money


• Withdraw money, honor daily withdrawal limit.


• Check the balance

This is what I did on my own, and I was getting rejected.

import java.util.Scanner;

public class CreateAccount 
 int accountid;
 String accountantname;
 String IFSCcode;

 public CreateAccount(int accountid,String accountantname,String IFSCcode)
 this.accountid = accountid;
 this.accountantname = accountantname;
 this.IFSCcode = IFSCcode;
 

 /*//adding deposit money with the balance 
 public double despositMoney() throws MiniumAmountDeposit

 double Currentbalance = 0.00;
 Scanner scn = new Scanner(System.in);
 System.out.println("please enter the deposit amount");
 double Depositamount = scn.nextDouble();
 Currentbalance += Depositamount ;
 System.out.println("your currentbalance="+Currentbalance);

 return Currentbalance;
 */

 //withdrawl money and set daily withdrawl limit
 public void WithdrawMoney() throws InsufficientBalException, MiniumAmountDeposit


 double Currentbalance = 0.00;
 Scanner deposit = new Scanner(System.in);
 System.out.println("please enter the deposit amount");
 double Depositamount = deposit.nextDouble();
 Currentbalance += Depositamount ;
 System.out.println("your currentbalance="+Currentbalance);

 /*double Currentbalanace = despositMoney();*/

 //setDaily Withdrawl limit
 final double setDailyLimit = 2500.00; 

 Scanner withDraw = new Scanner(System.in);
 System.out.println("please enter the withdraw amount");
 double WithdrawMoney =withDraw.nextDouble();


 if(Currentbalance < WithdrawMoney)
 System.out.println("you have less amount : your current balance is="+Currentbalance);

 else if (WithdrawMoney > setDailyLimit)
 System.out.println("you have entered amount exceed than daily limit : your dailyLimit="+setDailyLimit);

 else 
 Currentbalance -= WithdrawMoney;
 System.out.println("your current balance is="+Currentbalance);

 


 /*public void setWithdrawlLimit()throws exceedLimit
 Scanner scn = new Scanner(System.in);
 System.out.println("please enter the withdraw amount");
 double enterAmount = scn.nextDouble();
 double DailysetLimit = 2500;

 if(enterAmount>DailysetLimit)
 System.out.println("you have exceed daily limit : your dailyLimit"+DailysetLimit);


 */

 public String toString()

 return "accountid="+this.accountid + "accountantname="+this.accountantname + "IFSCcode="+this.IFSCcode;
 



 public static void main(String args)
 CreateAccount account = new CreateAccount(1234455533,"samy","ICIC09");
 System.out.println("you have created account : " +account);
 /* 
 try 
 account.despositMoney();
 catch (MiniumAmountDeposit e) 
 // TODO Auto-generated catch block
 e.printStackTrace();
 */

 try 
 account.WithdrawMoney();
 catch (InsufficientBalException 

I'm sorry to hear you got rejected - that's never any fun. I'll go through line by line, with some general comments at the end.

Some of my notes are labeled as style suggestions; these usually mean generally accepted Java principles (naming conventions, etc.), so I'll try to point out where I'm trying to argue for a given style I personally recommend instead of giving a general rule.

import java.util.Scanner;

public class CreateAccount 

Style: classes should usually be nouns - e.g. Account or maybe AccountCreator, because you use them as Objects - so it makes more sense to have "an Account" than "a CreateAccount"

 int accountid;
 String accountantname;
 String IFSCcode;

Style: Java almost always uses Camel Case naming, so accountid should be accountID and accountantname accountantName, etc.

 public CreateAccount(int accountid,String accountantname,String IFSCcode)

Style: method names should start with a lower case letter EDIT: this is a constructor (my bad) and is therefore correctly named. Non-constructor methods should start with a lowercase letter.

 this.accountid = accountid;
 this.accountantname = accountantname;
 this.IFSCcode = IFSCcode;
 

 /*//adding deposit money with the balance 

What is this comment supposed to tell me? The function is called depositMoney, so I expect that it will deposit money - you don't need to tell me that. Instead, it might be helpful to explain that the deposit amount comes from the user's input, or what the return value means

 public double despositMoney() throws MiniumAmountDeposit

 double Currentbalance = 0.00;
 Scanner scn = new Scanner(System.in);
 System.out.println("please enter the deposit amount");
 double Depositamount = scn.nextDouble();
 Currentbalance += Depositamount ;
 System.out.println("your currentbalance="+Currentbalance);

 return Currentbalance;
 */

This method is a little busy - it would be nice if the functionality to get input from the user was somewhere else (in a separate function), especially because you need to use it for multiple commands.

Also, it really seems like Currentbalance (currentBalance, if we use Camel Case ;p) should be an object variable.

In the question paper they clearly mentioned that no data persistence is required.

BUT, I think what they meant is, no data persistence is necessary between program executions (you don't need to save account data to a file when the program shuts off, and read it back in when it starts up again). It would make a lot more sense to keep track of a user's balance while the program is running, and check, for example, that the user doesn't withdraw more money than they have in their account.

 //withdrawl money and set daily withdrawl limit
 public void WithdrawMoney() throws InsufficientBalException, MiniumAmountDeposit


 double Currentbalance = 0.00;
 Scanner deposit = new Scanner(System.in);
 System.out.println("please enter the deposit amount");
 double Depositamount = deposit.nextDouble();
 Currentbalance += Depositamount ;
 System.out.println("your currentbalance="+Currentbalance);

 /*double Currentbalanace = despositMoney();*/

See the above note - we probably don't want to force the user to deposit money before every withdrawal.

 //setDaily Withdrawl limit
 final double setDailyLimit = 2500.00; 

This is a configuration constant - it should go at the top of the class, before methods. If all users should have the same withdrawal limit, it should be declared static, too.

 Scanner withDraw = new Scanner(System.in);
 System.out.println("please enter the withdraw amount");
 double WithdrawMoney =withDraw.nextDouble();


 if(Currentbalance < WithdrawMoney)
 System.out.println("you have less amount : your current balance is="+Currentbalance);

 else if (WithdrawMoney > setDailyLimit)
 System.out.println("you have entered amount exceed than daily limit : your dailyLimit="+setDailyLimit);

 else 
 Currentbalance -= WithdrawMoney;
 System.out.println("your current balance is="+Currentbalance);

 

These checks are good, nice job.

 /*public void setWithdrawlLimit()throws exceedLimit
 Scanner scn = new Scanner(System.in);
 System.out.println("please enter the withdraw amount");
 double enterAmount = scn.nextDouble();
 double DailysetLimit = 2500;

 if(enterAmount>DailysetLimit)
 System.out.println("you have exceed daily limit : your dailyLimit"+DailysetLimit);


 */

I don't understand what this function is for.

 public String toString()

 return "accountid="+this.accountid + "accountantname="+this.accountantname + "IFSCcode="+this.IFSCcode;
 



 public static void main(String args) MiniumAmountDeposit e) 
 // TODO Auto-generated catch block
 e.printStackTrace();
 

 /*try 
 account.setWithdrawlLimit();
 catch (exceedLimit e) 
 // TODO Auto-generated catch block
 e.printStackTrace();
 

In general, you shouldn't leave auto-generated TODOs in submitted code. For an interview, I would be fine with leaving TODOs explaining functionality I wish I could implement but didn't have time for.

*/ 
 

And that's the end, so, overall: It looks like you have a solid start on the problem. The biggest issues I see (in order of importance (my opinion) for code turned in as part of an application):

1. The code looks a little rushed. Just like in a face-to-face interview, appearances matter for code that will be read and reviewed. Big chunks of commented code with no explanation, as well as misspelling / inconsistent naming schemes, won't make a good impression.


2. It seems like you're a little unsure of the problem specification. It can help a lot to think carefully about the input / output to each function, and what data will need to be stored in object variables, as you're writing your code - these things can change as you go along and understand the problem more. It's very helpful to document (write in comments!) the assumptions you're making about how the program should run.


3. There are some spots where you deviate a lot from established Java rules. No company has the exact same code style (here are Google's for Java), but there are some things (camel case, for example) that are very central to a given language. Not using camel case correctly when writing Java is a signal that you don't know Java very well - depending on the position you're applying for, this may or may not be a problem.

My biggest suggestions for improvement are: always submit the cleanest code possible, try to follow broad, important, rules for how the language should look, so it looks clean to the reviewer, and work on writing short, helpful comments around tricky or interesting spots in the code.

Sorry to read that you got rejected. Let's try to learn form that.

First of all, try to be consistent in your names and follow the naming conventions. You have a WithdrawMoney method
inside a CreateAccount class. A 'command' class like your CreateAccount has usually on method that match the intent
(execute, perform, create). The methods and parameters must start with a lower case letter, IFSCcode should be
ifscCode and WithdrawMoney must be withdrawMoney. This is for the conventions and basic good practices.

double is not the best choice when dealing with money, you should better use BigDecimal. But in the ideal world you
may create a Money(unit, amount) type.

Separation of concerns

Your code mix the business concerns with the infrastructure, reading from the command line has nothing do to in the same
class as the one that manage an account. You should have a look at some commons patterns like MVC.

I would have introduced one AccountsService with one method for each use case :
+ Create an account: create():AccountId seems strange but since there are no requirements for creation, we keep it
simple, see YAGNI.
+ Deposit money: deposit(BigDecimal, AccountId):void
+ Withdraw money, honor daily withdrawal limit: withdraw(BigDecimal, AccountId):void ^DailyWithdrawLimitExceededExecption
+ Check the balance: getBalance(AccountId):BigDecimal

This service will be used to isolate the business logic. On the other side, a BankingApplicationCli may be used to
work with InputStream and OutputStream in order to invoke methods on AccountsService and print the results. Mine
will be a simple class with a main loop. The first step would be to select or create a account, then all actions will be
done on this account, like an ATM.

Modeling business concepts

I like to do a development that is close to DDD; I would have introduced one Account which can be retrieve or saved
from an AccountsRepository. This Account would have one method for each use case with some pre-conditions:

class Account 
 public final AccountId id;

 void deposit(BigDecimal amount) 
 assert amount.signum()>0;
 // ...
 
 void withdraw(BigDecimal amount) throws DailyWithdrawLimitExceededExecption 
 assert amount.signum()>0;
 // ...
 
 BigDecimal getBalance() 
 // ...
 

You see that there is a direct match between the methods from Account and AccountsService. The service will just
enforce the preconditions by validating the inputs and delegate to the methods on Account. This is where you do the
link between the infrastructure and business (repository is infrastructure but later you maye also add security,
transactions, logging, ..)

class AccountsService 
 private final AccountsRepository repository;

 void withdraw(BigDecimal amount, AccountId accountId) throws InvalidAmountException, DailyWithdrawLimitExceededExecption 
 if ( amount.signum()<1 ) 
 throw new InvalidAmountException(amount);
 
 Account account = repository.get(accountId);
 account.withdraw(amount);
 repository.save(account);
 

 BigDecimal getBalance(AccountId accountId) 
 return repository.get(accountId).getBalance();
 
 

Notice that I use a conventions that use verbs for method that changes the system but looks like getters when querying
the state. You may not like this conventions but the idea is to highlight the importance of conventions and self
documented code.

Designing the deposit and withdraw methods

The first idea that I had in mind was to manage one mutable BigDecimal field. It works well for the basis of deposit
: this.balance = balance.add(amount) and withdraw. But then when you have to deal with the daily limit, things are
more complex. You may hold a date and a sum of daily withdraw but another way is to maintains a List of Operation
that have a time and an amount, deposit will add a new Operation(amount) while withdraw add one
new Operation(amount.negate()).

To compute the daily withdraw, you have to filter all the operations for the current day that are withdraw
(amount is negative) and sum them. The balance is computed by reducing all the operations via an addition.

public BigDecimal getBalance() 
 return history.stream()
 .map(e -> e.amount)
 .reduce(BigDecimal::add)
 .orElse(BigDecimal.ZERO);


/**
 * Decrease this account balance of the given amount
 * @param amount a positive amount of money to remove from this balance. NotNull, GreaterThanZero
 * @throws WithdrawLimitExceededException if the daily withdraw limit is exceeded. Can be checked
 * via getWithdrawAmountOfToday()
 */
public void withdraw(BigDecimal amount) throws WithdrawLimitExceededException 
 assert amount.signum()>0 : "withdraw amount must be positive";
 BigDecimal foreseenDailyWithdraw = getWithdrawAmountOfToday().add(amount);
 if ( getDailyLimit().compareTo(foreseenDailyWithdraw)<0 ) 
 throw new WithdrawLimitExceededException(getDailyLimit());
 
 history.add(new Operation(amount.negate()));

Testing

By separating everything you can easily test the behaviors of Account and interactions of AccountsService. The only
things that is complex to test is your BakingApplicationCli.

You can find my solution on https://github.com/gervaisb/stackexchange-codereview/tree/188306/src/main/java/q188306. I have also added one AccountFactory to create a new valid Account. This one
does not do many thing but may evolve in time and isolate the creation process. I have chosen to add a getDailyLimit
method in Account so that we can imagine to have on abstract Account and many implementations with different limits.
The factory is then the perfect place to choose the implementation.

Just something other did not mention and is quite important:

The awareness of concurrency which can corrupt non-atomic transactions. Especially deposit/withdraw is very typical, fishing for mentioning concurrency issues. Atomic<BigDecimal>

As doubles are imprecise, better use BigDecimal: a double 3.1 is the same as 3.1000 but might actually be 3.0999994.

However as there are several things other people already mentioned, I leave it to that. For a fast improvement:

• Follow java conventions, especially w.r.t. naming: methods and variables start with a small letter.


• Separate human input/output from the methods on data objects.


• Pay attention to spelling ("desposit"). Nobody wants his beatiful code extended with identifiers like serachKey (often seen).


• Think in data structures, if one has an Account class, add a BigDecimal amount = new BigDecimal("0.00"); to it.

My two cents.
So the intention I understand was to create a Bank Application with certain banking actions. What I feel is that, that code you have shared is not an application but a program which can do the banking actions mentioned. One way to see it was to start thinking from an End Users perspective.

• How my users will use the application ? Will they use an API ?

If it has to be an API, then you could choose it to be a Spring Boot Microservice. Since it will be the simplest alternative.

Then the next would be the architectural design of your application.
The banking actions mentioned in the above question are your "Domain".

The Domain's requirments needed to be encapsulated from the rest of the world. (In their own class and package)
The Domain's requirments would need functionality / integration tests coverage.
There need to be Unit Tests coverage as well for the Services that you would create.

Finally, you might want to specify how will this Application be deployed.
Say using Docker or Kubernetes. By that I mean how will you ensure fast delivery of the changes to the Microservices Application.

These are some of the key things I feel they would expect when they want you to create an application.

In addition to MyStackRunnethOver's excellent answer (especially issue #3, Coding Standards):

Should you wonder, why it is good practice to adhere to Coding Standards, just consider the following:

Try to figure out, when the last System.out.println is executed in your code:

if(Currentbalance < WithdrawMoney)
 System.out.println("you have less amount ..");

else if (WithdrawMoney > setDailyLimit)
 System.out.println("you have entered amount ..");

else 
 Currentbalance -= WithdrawMoney;
 System.out.println("your current balance is ..");

You'll see that the last System.out is outside your if .. else if .. else and is executed in any case.

This is at odds with your indentation, which suggests, that it should belong in the else block.

This mismatch between intended behaviour (expressed by the indentation) and actual behaviour is rather likely to raise eyebrows.

Do yourself a favour and get used to using braces:

if(Currentbalance < WithdrawMoney) 
 System.out.println("you have less amount ..");
 else if (WithdrawMoney > setDailyLimit) 
 System.out.println("you have entered amount ..");
 else 
 Currentbalance -= WithdrawMoney;

System.out.println("your current balance is ..");

(e.g. as described in Google's style guide).

This makes the intention of your code a little clearer..