aspose file tools*
The moose likes Java in General and the fly likes Trying to create OO based bank app Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Spring in Action this week in the Spring forum!
JavaRanch » Java Forums » Java » Java in General
Bookmark "Trying to create OO based bank app" Watch "Trying to create OO based bank app" New topic
Author

Trying to create OO based bank app

Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
I suspect you will find passing an Amount to the Account#withdraw method might be the mos OO approach.
You might do well to have canWithdraw methods.Obviously your version will be slightly different.
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
Yeah account class all sort of that methods: deposit , withdraw , interest etc....

Single Responsibility: a class takes care of itself and does its own actions.

In this case option 3 is valid but i will think again..


You might do well to have canWithdraw methods...... Obviously your version will be slightly different.

Yes, i am also thinking the same..
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
My Account class:

fields: account id, account status, balance, penalty amount, interest rate, minimum balance to maintain, overdraft amount
behavior: isAccountActive, isWithdrawPossble, isMinimumBalanceMaintained, deposit, withdraw, penalty , makeAccountInActive, interest, closeAccount, storemaximumUsedAccountIdIntoFile


Transaction:
field: amount
behavior: getAmount


I also reading about SOLID again to get more idea...
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
The names of those methods should match the bean pattern. It should be isActive and setActive, etc etc.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
Are you sure the storeMaximumID method belongs to the account class? Might it be a function of the bank class?

And, good idea to find out about SOLID but be aware that many people no longer believe the O bit.
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
Are you sure the storeMaximumID method belongs to the account class? Might it be a function of the bank class?

No sorry, i have mentioned wrongly as it is not write operation but read maximum current account id from file and write is definitely part of Bank class as we did in Customer class...


And, good idea to find out about SOLID but be aware that many people no longer believe the O bit.

Yes thanks again..
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
Sorry, i took long time to understand SOLID but i am feeling better now(for SOLID)

I going to redesign my Transaction and account class now. I am making account class abstract Defining several interfaces in account class so that as per account type
characteristics particular interfaces can be implemented.

Transaction


Account Interfaces To follow Interface segregation principle

Interest


Penalty


Minimum balance


maximum overdraft


Account class Open closed principle is broken here


Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
Looks good

But how are you calculating interest on an account?
Why do your setXXX methods return a BigDecimal?
Do you have any constraints about the values of those BigDecimals? Why are you not doing any validation in the transaction constructor?
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
Why do your setXXX methods return a BigDecimal?

Sorry, wrong one. I have corrected it. Instead now, i think let the default value passed in my abstract class account and provide setter methods. So any specific
account which implements specific interfaces can update the value to this variables as per need.

Do you have any constraints about the values of those BigDecimals?

Yes, amount should be greater than 0

Why are you not doing any validation in the transaction constructor?

Corrected

But how are you calculating interest on an account?

I thought interest calculate per day basis and credited it to customer account quarterly basis. Any account which provides interest must implement Interest interface which set interest rate.
Each subclass also have method addDeposit() or recoverPenalty() which add interest rate or subtract penalty from the user account.


Transaction class:


Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
I having a one doubt.. If for any account i set interest rate or penalty or other thing then that changes will be reflect in all the accounts and this is not desired.
I tried to think the solution but could not able to solve it.What i am thinking to make all the methods abstract and any class which implements these methods needs define it.
Or to override some particular methods like withdraw.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
You cannot make a method abstract and then have subclasses requiring it implement it. Otherwise classes not requiring that method would have to be abstract.
What you can do is have methods in the superclass like this and override them. In Java8 you can even have such methods as default implementations in an interface:-You should decide whether you have types of account all with the same interest rate. That is common, actually. In that case you would consider having interestRate as a static field.
Beware: You probably need a separate static field for each class, because of hiding.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
Transaction looking good, but I have realised there is something I forgot. Sorry. A Transaction may have an amount and a transaction Date or Time.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
Tushar Goel wrote: . . .

Transaction class:


If the transaction is a withdrawal, that will represent the amount given to the customer. Just say, “amount of this transaction.”
Is that class intended to be final? If not, I suspect it should be abstract with classes like Deposit and Withdrawal as subclasses.
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
sorry for late response, i was away due to personal issues..

What you can do is have methods in the superclass like this and override them

Thanks...
with classes like Deposit and Withdrawal as subclasses.

Just wondering why we need to extend Transaction class and have deposit and withdraw as subclass.. I thought they are methods of the Account class...
Is it because of single responsibility principle ?
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
That way the Account can maintain a List<Transaction>. There would be other ways to do it.
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
That way the Account can maintain a List<Transaction>. There would be other ways to do it.

Thanks.. I was unable to think this.. I need more practice...

As suggested i have made changes:

TransactionType


TransactionClass



Deposit


Withdraw
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
Sounds good. Now you can have takeTransaction methods in Account, or you can have withdraw and deposit methods.
What about transactions for charges or interest?
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
What about transactions for charges or interest?

Yes, they deserve separate class as well but i think charges due to not maintaining minimum balance and interest calculation done on quarterly basis so i think
this will be done some scheduled thread and can not calculate in current application. For charges due to overdraft can only be implement as of now.

I also made some changes, i thought instead of having Transaction interface let only abstract Transaction class there and other classes will extends it. Also i think i should
add TransactionId as well for uniquely identify each transaction object by overriding equals and hashcode method.

Charges


Account class This is my transaction class.. I think there is one 1 thing left here. saving the list of transaction in file. I think i need to implement Serializable interface to the Transaction class. I will
do it once rest piece of code is fine

Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
Good grief! You didn't really write == true, didyou?
That method should be called setAccountActive(boolean active)
Agree that serialising is a simple way to maintain account details. The banks don't do that; they maintain the details on a database server behind foot‑thick steel doors. Actually multiple servers several miles apart. And they don't let anybody find out where those servers are.
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
You didn't really write == true, didyou

Actually, i did.. Sorry i dint realized it... I made correction..


Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
Caught you Actually I preferred the version with the :? that you had earlier.
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
Modified it:

Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
Modified? Worsened, you mean. Apart from the == true, what was wrong with the first version?
Similarly the isAccountOpen method can be simplified greatly.
The names of the methods should correspond: matched setXXX and getXXX methods should have the same XXX
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
And what will happen to a closed account if you call setActive(false) on it?
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
if account is closed then nothing will happen. I will put this check in bank class so that user can not go further and get warning then and their..

That method is in Account class but will call from the bank class
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
That check should be in the Account class.
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
but i thought invalid input should not come to Account class at first place. Is it ok to put double check on it? If that's so i need to put a check in each method because if account is closed then it again request for deposit and withdraw or others...

Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
Reposting again, i think due to reediting previous post goes un-noticed.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
Sorry for missing it.
Better to check twice than not to check at all. Yes, you may have to throw that Exception from each method. You can write a private method which checks that the account can receive a transaction and throws that Exception.Decide whether the testAccountAvailable method throws the Exception from closed accounts or inactive accounts or what.
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
Thanks. I have made recommended changes now..

Decide whether the testAccountAvailable method throws the Exception from closed accounts or inactive accounts or what.

It will throws exception for closed accounts.

This is my updated Account class:

Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
Tushar Goel wrote: . . .
It will throws exception for closed accounts.
. . .
But I can't see any Exceptions in the set active method. Why are you using Booleans and compareTo?
I presume you are going to override the mechanism for calculating penalties in subclasses; it looks a very complicated way to get 0. I would prefer to see all fields initialised in the constructor (others will disagree with me), in which case you would want only to call private methods or final methods from the constructor.
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
But I can't see any Exceptions in the set active method.

Oops sorry, i forgot to put it. Corrected now.. Thanks for pointing..


Why are you using Booleans and compareTo?

Because i earlier used, ( isActive == true ) and after your pointing i changed it to if-else but that one too not looks good. So i used this. Is this again wrong?

prefer to see all fields initialised in the constructor

You are right. It is much cleaner and eaiser approach then previous one. Is this one you referring to? Also if its ok, i think then i should use builder pattern as number of fields to be initialized are too many.



Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
And what is wrong with
status = active ? Status.ACTIVE : Status.INACTIVE;
?
Junilu Lacar
Bartender

Joined: Feb 26, 2001
Posts: 4710
    
    7


There are a few things that are not quite right with this code.

First, the comment includes a description of logic that does not exist at all in the code. That is, despite what the comment says, there is not one line of code in the method that examines the account balance and determines "sufficiency" of that amount before setting the status.

Second, passing in a boolean parameter is a code smell that indicates a method that should probably be split into two methods. So instead of setAccountActive(boolean), prefer having two methods instead: activate() and inactivate() - this makes the code more focused, that is, each method does one and only one thing, and it makes the semantics clearer. Compare setAccountActive(true) vs activate() and setAccountActive(false) vs inactivate().

As for the "rule" that involves account balances and "sufficiency", I would try to define an AccountRule, perhaps as an interface, like so:

Of course, there would be other variations to this, depending on how complex your rule set is. For example, you may want to only do something if at least one rule in a set of rules is satisfied. Or do something only if all rules in a given set are satisfied.


Junilu - [How to Ask Questions] [How to Answer Questions]
Tushar Goel
Ranch Hand

Joined: Dec 29, 2013
Posts: 254
And what is wrong with

My goodness.. It is very simple but i could not able to think of this. I slapped my forehead.

Junilu
First, the comment includes a description of logic that does not exist at all in the code.

Sorry. Actually i made several correction in my account class since i made though i forgot to update comments as well due to lack of practice... I am sorry again.
I will keep them updated as well as soon as i update the method logic.

a method that should probably be split into two methods.

Thanks it looks more cleaner approach then i was doing.

As for the "rule" that involves account balances and "sufficiency", I would try to define an AccountRule, perhaps as an interface, like so:

Thanks a lot. It is very good example better than i can able to thought. I thought to make individual interface for each rule like for MinimumBalance, InteresetRate, Penalty etc.. and let the subclass of
Account class implement them but that one too not worked. So we (Campbell and I) discussed to let this initialization do using constructors.
So now i think i should add both of your advice into one Like let the initialization done through constructor and in the method do check using the individual classes
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
Tushar Goel wrote: . . . i could not able to think of this. . . .
Really? It is what you wrote earlier minus the == true bit. Well, as near as I could remember it, anyway.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
Junilu Lacar wrote: . . .
First, the comment includes a description of logic that does not exist at all in the code. . . .
I thought we had discussed reopening a closed account, in which case that method should throw an Exception.

Second, passing in a boolean parameter is a code smell that indicates a method that should probably be split into two methods. . . .
If you use the bean pattern, however, activate and deactivate methods would not fit. It would be setActivated(true) rather like setVisible rather than show and hide.
Junilu Lacar
Bartender

Joined: Feb 26, 2001
Posts: 4710
    
    7

Tushar,

Don't get down on yourself or apologize for not knowing any better. It's almost always the case that beginners tend to write more code than is necessary. In time and with more experience, you will learn how to write more concise code.

One of the best ways to get better at programming is to read other people's code. Look for code that's easy to understand rather than code that is "clever" - prefer clear and concise code over clever but convoluted code. Then of course there's elegant code that combines being clear, concise, and clever. Also, read a few style guides. Here's a search that brings up a few, including JavaRanch's own style guide: java style guide -- read these, try them out and see what works for you.

Junilu Lacar
Bartender

Joined: Feb 26, 2001
Posts: 4710
    
    7

Campbell Ritchie wrote:If you use the bean pattern, however, activate and deactivate methods would not fit. It would be setActivated(true) rather like setVisible rather than show and hide.

Either way is acceptable, IMO. I think it just boils down to the choice of names and the style you want to follow.

setActivated(false) and deactivate() are both preferable to setAccountActive(false). Once you have a name that makes sense, you just have to decide whether following bean conventions is more important than not passing in a boolean parameter. You could also reason that the semantics of setActivated(boolean) is clear enough and doesn't really smell, and you're following bean conventions to boot, to which I would probably say, "Sure!"
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39395
    
  28
OK
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Trying to create OO based bank app