Trying to create OO based bank app

Hello friends, i am trying to make oo based bank app for oo practice. It will be great if some one review or suggest something the initial requirement so that i can start working on it.


accounts: saving, salary, current

min balance per day:
saving: 20$
salary: 0$
current: 100$

operation: withdraw, deposite

user id: account number(12 digit integer), unique id(8 digit integer), password(10 digit alphanumeric)
is provided to the users who are having bank account.

Account opening: user should provide name , dob , address , ph. no and type of account it
wanted to open and minimum balance amount needs to be deposited. Once successfult bank provide account number, unique id, password

Account closed: Any time account will be closed if balance is 0 or more. All money will be refunded.
if balance is negative due to penality or any other reason then user first needs to
clear that. Once account is closed then its account number can not be reassigned.

General rule:

a) 1 user can have any number of accounts of any type

b) user can deposite any amount for any accounts it helds or for another user as well
but it requires account number and customer name for the account holder to which deposite needs to be made.

c) user balance can not be negative.

d) if any point user balance is less that minimum balance / day then penality of 10$ will be charged
every month.

e) transcation will be failed if user tries to over withdraw, user enter wrong info(account number, id or name)

Why are you thinking about operations? Why are you not thinking about things? Object programming means you think in objects. The operations come later. And constraints like minimum balance come much later.

Why are you not thinking about things?

Can you please explain this? I am sorry but i do not understand this. I thought things is what it is having like account details and what it will do like (deposit / withdraw). Sorry for asking small question.

You need to think much simpler than you are doing.

Operations: put money in, take money out.
Things: money, account, customer.

Write the things down first, because they will tell you what object you want to create. Then consider their operations later.

Thanks Campbell.. I understand what things are now... Important lesson...

As per that i come up with this details:

account: must have unique account number related to each account, type of account can be one of saving , salary, current(business account),
available balance, customer id to which it belongs

customer: unique customer id, name , mobile number, address, date of birth

money: can be of 1, 5, 10, 50, 100, 1000

Is it ok now?

You are still overthinking it.
Money can be 2 or 20 or 25 or 500. By putting your constraints on money you are making things harder than they need be. Money is money. You don't need to know more than that at this stage. I think you should simplify your thoughts a lot. You can add details later. All you need to know about money at the moment is that is has amounts.

Thanks Campbell, I will try to simply it.. What about account and customer, is it ok now?

What should i do next? Should i think about account and customer classes or should i think about operation?

Yes, a lot better.


Start with a bank class

Yes, a lot better.

That is encouraging..

Regarding Bank class: I thought Bank will serve lots of customer each having unique id and each customer can have multiple account and then each account also have unique account number.. Also as bank will provide deposit and withdraw facility to each customer. So, i thought class should have:

Bank Class:
State: Customer set
Behavior: takeUserDetails() , withdraw(), deposit(), openNewAccountByExistingCustomer(), openNewAccount(),
validateCustomerIdForExistingCustomerForNewAccount()

Customer:
State: customer details (like unique id , name, email etc) , account set
Behavior: updateCustomerDetails() , addNewAccount()

Account
State: accountnumber , accounttype, balance
Behavior: generateAccountNumber(), validateBalanceInCaseOfWithdraw(), updateBalance()

AccountType:
enum class having Saving , Current, Salary and BLANK


But still i am not satisfy by this. somewhere i thought that Bank only manages bank accounts, it doesnot care the customer. For each account it consider a unique
customer and it manages that only. If this is true then my bank class should have reference of both account number and customer.

bank class:
State: : Account , Customer
Behavior: takeUserDetails() , withdraw(), deposit(),

You need to decide whether a customer has an account or an account has a customer. It is not like writing database schemata where you might write both. You want one or the other.
Also does an account open another account?
Do you need “state”? Can you have an account which does not belong to anybody?

You need to decide whether a customer has an account or an account has a customer

Customer has a account

Also does an account open another account?

No, only customer can.

Do you need “state”?

If we require to save list of customer then it may be required. Otherwise do not required. We can save the same in a file or in a database.

Can you have an account which does not belong to anybody?

No, we cannot have. Every account has its account holder. Bank can have list of accounts which are not utilized or we not assigned to any one.

What i thought now, In Bank class no state is required, it should show menu(Open a account, make deposit or withdraw). If user selected open a new account then should receive inputs
from the customer, validate it and ask the customer and account class to generate customer id and account number. Once all this info received it should then store it into somewhere
(can be file, database or anything) and inform the user about new account opening confirmation. But if user selected some operation like deposit or withdraw then it validates the
customer id , account number and do some checks and perform activity and get back result to the user.

I do not mean whether there is any state in a Bank, but whether the state field describes above does anything useful. If it is simply noting whether a Customer has an Account or Accounts, it can be calculated from seeing whether account is null or not.

Yup you are right. The state does not add any value .

So Bank class looks like this:

state:
Behaviour: showMenu() , takeUserDetails(), validateDetails(), createNewAccount(), storeAccountInformation(), updateCustomerDetails(), withdraw(), deposit()

Do you think it is correct or am i still misunderstood? I am sorry but i am not fast learner.,.

What fields have you got in the Bank class?

actually no field..

It will call methods(getter/setter) from the Customer class to create a customer id and store the customer info and then call method from the customer class to
generate account number , store account type and balance from account class.

Is it right thing or wrong?

or I also thought to have map field<Customer, Account> which store all the customer info and their related account info but this will not be persistent. For saving the data
i thought not to use map, instead write customer object into the file itself so that if user again comes back then it can see old values.
I know map option is better option but is there any other alternative for the same?

I am very much confused here.. Can you please suggest the way or give some hint?

What on earth makes you think you can have a bank with no data? Of course a bank will have fields. The Map might be a field.

But you are getting ahead of yourself thinking about persistence. Do you know any database programming?

What on earth makes you think you can have a bank with no data? Of course a bank will have fields

Thanks Campbell, I was thinking in wrong direction...

To come up this i asked few questions to myself. Do not know whether they are correct but i think they are important

What does bank do? Providing some services to the customer
who are the customer? Those having accounts with them
How it linked with customer? To linked with the bank, customer should have account with it.
Does bank manages customer or accounts? accounts
How it identify customer? using unique customer id
What if user have multiple accounts? how it identify ? by their unique customer id

If above questions and their answer are right then I think Bank class should have following fields:

Map<CustomerId, Customer> customers , Map<Accounts, CustomerId> accounts

Do you know any database programming?

Yeah, i have some knowledge.

The ideal way to persist things is to put them into a database.

Yeah, right.. That's why i initial thought not to have any field and directly stored them in some database..

But any way if i need to use database i should not required any map then.

I can have then Bank class looks like:


State: Customer , CustomerId, Account
behavior: showMenu() , takeUserDetails(), validateDetails(), createNewAccount(), storeInformation(), updateCustomerDetails(), withdraw(), deposit()

Is it correct now? Should i start creating class or any other changes required?

I will start creating tables as well if above is right..

Unless you can do all your processing in the database, you will need fields somewhere. If you can do all your processing in the database, you might get away with your app being stateless. But then all your methods will have to turn into functions or procedures and you are no longer writing OO code.

your methods will have to turn into functions or procedures and you are no longer writing OO code.

Thanks, i clear now... now i understand that i was thinking like procedural language. Sorry due to old procedural language background it happened...

BankClass:

fields: customer , customerId, accountNumber, account, accountType, name , address, dateOfBirth, initialAmount
behavior: showMenu() , takeUserDetails(), validateDetails(), createNewAccount(), storeInformation(), updateCustomerDetails(), withdraw(), deposit() , getCustomerId(), getAccountNumber()


Customer:
State: customer id, name , address, dob, account
Behavior: updateCustomerDetails() , addNewAccount(), generateCustomerId()

Account
State: accountnumber , accounttype, balance
Behavior: generateAccountNumber(), validateBalanceInCaseOfWithdraw(), updateBalance()

AccountType:
enum class having Saving , Current, Salary and BLANK

is it correct now?

The customer will have a date of birth, the bank probably not.
The bank doesn't have an initial amount, but an account might. You could even make it static and use it as an advertising ploy: open an account during July 2014 and receive $10 free. I shall leave you to work out how to do that. It is easier than you think, but it is something you should leave until you have got everything else working.
You are still over-thinking the whole thing and making it too difficult for yourself. I think you have too many fields in each class.

Decide the fields for each class and forget about the methods. You can add the methods later.

Thanks Campbell... Actually i added earlier fields in BANK class to store customer info(dateOfBirth , initialAmount, name etc are belongs to the customer not to the bank)

Now, I have removed them from the Bank class.

BankClass:
fields: customer , account

I think you have too many fields in each class.

I think i need to store customer and account info that's why i added these many fields.. By using this fields i can get the values back when i am going to store info into the databas. Am i thinking right or still wrong?

Customer:
State: customer id, name , address, dob

Account
State: accountnumber , accounttype, balance

AccountType:
enum class having Saving , Current, Salary and BLANK

Surely that would read customers, accounts in Bank? Your fields look all right otherwise.

You may wish to create a Transaction class too.

Surely that would read customers, accounts in Bank?

Thanks Campbell... I thought earlier after we have stored data in to the database we do not required that that's why i have used plural term. But now i thought that if i used customer(s) and account(s) then i am able to achieve 2 things.
1) maintain state 2) faster access. I need not to check my database again and again. is it correct or some other reason?
I think then field should be:

Bank:
field: Map<AccountId, customer> accountLinkedWithCustomer, Map<AccountId, Account> account
behavior: showMenu() , takeUserDetails(), validateDetails(), storeInformation(), updateCustomerDetails(), getAccountNumber()

Transaction:
field: accountId, customerName, customerId, amount
behavior: verifyAccountId(), verifyCustomerID(), verifyAmount(), verifyName(), deposit(), withdraw()


Is it fine now? If yes should i start coding???

Not convinced about the Maps. That is how you would link things in the database, but in a Map you would link a customer to an account. (Or maybe to several accounts in a Collection.)
And you would link an account to a costumer (or maybe several customers in a Collection).

See how you can use a Map to implement that sort of thing. Find out about bidirectional maps; I think there is an implementation you can download from Apache Commons.

you would link an account to a costumer (or maybe several customers in a Collection).

But i think a account should link to single customer only except if it is joint account... Then why it is required? I am sorry but i failed to understand this....

Find out about bidirectional maps;

As per interface description in bidir map key and values should be unique. UnLike normal map, in BIDIR map multiple key can not have same value.
In such case a customer cannot then have multiple account with same customerId. User needs to use different customer id for each account.
I think it is should not be happened. Ideally Customer can use same id to open multiple accounts. but as this is dummy app and instead of taking
customerID from the user we ourself generating it then each account holder get it is customer id irrespective
of how many times a user requested to open an account but then we cannot say that a customer has many accounts?

Campbell, what do you think about it??

You have answered your own question. An accoiunt might link to several customers if it is a joint account. You appear to have explained that you think a bidirectional map is not suitable for this current exercise.

It is not a case of

Campbell, what do you think about it??

but

Tushar, what do you think about it??

You are building this app, not I.

You are building this app, not I.

Sorry Campbell, if its sounds wrong..

Yes we can use bidir map in my application as we are generating customer id ourself. If we take this value from the user then we cannot use that.
Also i realized that we are not considering the case which i thought and told you earlier...Sorry again..

I am reading bidirectional map in more details will get back to it. so final Bank class looks like:

field: BIDIMAP<Accounts, Customer> accounts
behavior: showMenu() , takeUserDetails(), validateDetails(), storeInformation(), updateCustomerDetails()

Is it fine now, if yes should i start coding?

No, it wasn't that what you said sounded wrong. But you need to make decisions yourself.
As for bidirectional maps. You appear to have found that they are not suitable, but you would take the customer details before creating a Customer object, so you can use the values taken from the user.
But remember that a Map depends on the hash codes of its “K”s not changing. Presumably in a bidirectional map the “V”s' hash codes must not change either. So you may have to have a Customer reference object and an Account reference object which encapsulate a customer ID or account ID. Then they would have constant hash codes.
You may find the object creation process fraught in that case, if you are to avoid a this reference escaping before the constructors complete. I shall let you look up that problem. And at this point it ceases to be a beginner's exercise, so I am moving the discussion.

Thanks Campbell for insight... i designed Bank class now, Please suggest if it is fine.

What testing have you done?
Why are you passing Scanners around? Why don't you design yourself a utility class to handle keyboard input with Scanners? If you do it right you can obviate any Exceptions.
You have an infinite loop with no exit. Why? Beware: Sysyem.exit can be dangerous. It is a bit too vicious for many uses. If you exit, are you closing the app altogether or are you storing its state anywhere?
What does your Scanner point to? Are you closing a Scanner pointing to System.in? Never do that.
You seem to have methods with parameters never used.

Thanks Campbell, i will work on it and come back once done... Thanks again..

Javadoc of your method validateInput(String, String) says it checks whether its first argument is null.
What it really does is checking if a String is empty. It is not the same as being null.

Your javadoc says that the method will throw IllegalArgumentException if the first argument is null. But it will not. It will throw NullPointerException.

Also, the javadoc says that the method will return true if the string is valid, but it is declared as void.

Sorry for nitpicking but it is very important that your code does what your documentation says it does. Especially when you are working with other programmers.

This might be considered cosmetics, but you don't have to use .length() == 0. You can use .isEmpty().

Paweł Baczyński wrote:
This might be considered cosmetics, but you don't have to use .length() == 0. You can use .isEmpty().

There is a pitfall here, unless of course you know about it*. Both the length() and isEmpty() methods consider white spaces as legal. So if you have something like String name = " "; the length() would return 1 and isEmpty() would return false. Thus it is always advisable to invoke a trim() before length()/isEmpty()

*This once cost me a few hours to learn.

but it is very important that your code does what your documentation says it does. Especially when you are working with other programmers.

Thanks Pawel for correction and i will correct it.

Thus it is always advisable to invoke a trim() before length()/isEmpty()

Thanks Maneesh for suggestion and i will correct it


Thanks again both of you..

I have modified by bank class and also create utility class. Is it correct now?






MyUtilities class:



/**
*
*/

BakTest Class: