Trying to create OO based bank app

That means if user enters 06-01-1986 and if return back is 6-1-1986 then it is fine.... Oh great ....

Now if you think everything is fine then let us move towards Customer class....


Customer Class:

state: customerName, dateOfBirth, customerId, customerIdGenerator, CUSTOMER_ID_START_FROM
behaviour: getCustomerName(), setCustomerName(), getDateOfBirtth(), getCustomerId(), equals() and hashcode()

I also thought that in Customer class , customerId should be saved some where so that if program is terminated for some reason then it should assign customer id greater than the already
assigned. It should not start again from the initial starting level.

Due to this i am as of now storing maximum customerid used into the file when program terminates. To do the same i again defined another utility which is reading and writing into the file.

Modified Customer class:

Customer Class:

state: customerName, dateOfBirth, customerId, customerIdGenerator, CUSTOMER_ID_START_FROM
behaviour: getCustomerName(), setCustomerName(), getDateOfBirtth(), getCustomerId(), equals() and hashcode(), initializeCustomerId()

Campbell, just wondering if you got time to look at my last 2 posts?

Tushar, please EaseUp (<- click link and read). You are getting some incredible help here from Campbell but please do remember that we are all volunteers and have other demands on our time, like actual paying jobs, family, hobbies, whatever. Remember PatienceIsAVirtue so sit tight, I'm sure you'll get a response in good time.

Sorry Tim.. I completely understand your views and having completely respect to Campbell.. He already supported me very much and cannot explain thanks to him in words..

You have a dreadfully incorrect version of equals().

Thanks Campbell, i dont know what i thought at that time to create such method.. I just did some unit testing to see what error will i get if i use incorrect one and things is clear to me now..

Corrected one is as below:

Tushar Goel wrote:Corrected one is as below:

This is almost correct.
There is one problem. You attempt to cast obj to Customer before you check that you can do this.

Thanks Pawel, I have corrected that one too now..

That makes the hash code method incorrect. What will happen if you have this?
Customer:id=1234567890 name=Campbell Ritchie dob=12/3/45
Customer:id=1234567890 name=Tushar Goel dob=23/4/56

But i considering unique id.. No 2 customer can have same id...

I used following implementation to achieve the same:-



Is it wrong?? Also once application is down the maximum id used is being written in the file so that whenever it is up and id grater than already should be assigned:



Bank Class: To save already used customer id to the file




DataReadWrite Utility: Writing data into the file

Tushar Goel wrote:But i considering unique id.. No 2 customer can have same id...
. . .

We are trying to make you think about it.

If the ID uniquely identifies the Customer, why not use only the ID in the hash code method?

Understood Sir.. That's means it will help to provide unique value

So it's possible to have two customers with the same ID but different names?

It is not possible to have same ID for two customers but by adding extra checks i think it is making doubly sure to have unique object only.

I would suggest you confirm that you cannot get duplicate IDs, then you only need to use ID in equals and hash code.

You should always use the same fields in hash code and equals.

confirm that you cannot get duplicate IDs

I tried to do some testing and it always give me unique case.. I am unable to think about the cases in which customerId is duplicated(except the case when application is shutdown and restarted again or the case of
multithreaded.). Also while testing i realized that my current hashCode implementation is wrong as when i passed null to the fields it throws NullPointerException.. I need to correct that one too...

You should always use the same fields in hash code and equals.

Thanks for it. Earlier was not sure about it...

Tushar Goel wrote:It is not possible to have same ID for two customers

Yes, right, that's what I thought.

... but by adding extra checks i think it is making doubly sure to have unique object only.

In my opinion the equals() method is not the place to be making those "extra checks". If they aren't done correctly in the place where they should be done, then making your equals() method incompatible with your hashCode() method is just going to lead to more confusion.

If they aren't done correctly in the place where they should be done, then making your equals() method incompatible.

I tried to make them unique. I tested several cases on it to check and every time i am getting unique value. What other cases i need to test? Because if my implementation is
right then as Campbell suggested i need not to use anything else except customerID in equals() and hasCode().

I have added few more test cases to the existing one:

Tushar Goel wrote: . . . I am unable to think about the cases in which customerId is duplicated(except the case when application is shutdown and restarted again or the case of
multithreaded.). . . .

In which case you need to handle those conditions. I can think of three ways to handle the shutdown problem but one involves a database and we decided earlier not to use databases in this instance. Consider a shutdown hook on the runtime which does one of these things

1: Persist the state in the database.

2: Persist state in an XML or similar file.

3: Persist the state by serialising the bank to a .ser file.

Remember shutdown hooks only hae a short time to operate in. If you were using Swing, you could add a Listener to listen for the app closing.
If you are using AtomicInteger, doesn't that obviate any problems caused by threading? Have you checked?

It is a valid decision at this stage to decide to delay persistence until later in the proceedings, as long as you do it eventually.

Tushar Goel wrote: . . . my current hashCode implementation is wrong as when i passed null to the fields it throws NullPointerException.. I need to correct that one too...
. . .

And the way to correct it is to make sure a null never gets that far.

Tushar Goel wrote: . . . Because if my implementation is right then as Campbell suggested i need not to use anything else except customerID in equals() and hasCode().
. . .

You need to go through the constructor and tease it apart with pencil and paper and confirm whether there are any circumstances in which you can get a duplicate ID. I presume that is what you did earlier when you said shut-down or threading. In which case check what you did to confirm whether your conclusion is correct.
What if you have two Bank instances in different threads? Can you get duplicate IDs in those instances?

Consider a shutdown hook on the runtime which does one of these things

Thanks Campbell.. Let me read about shutdown hooks and get back to you. Also i think in shutdown hook i will save state into the file.

If you are using AtomicInteger, doesn't that obviate any problems caused by threading? Have you checked?

Yeah i did.. AtomicInteger is thread safe and it helps to maintain a unique customer id.

you did earlier when you said shut-down or threading. In which case check what you did to confirm whether your conclusion is correct.

As of now when the application is shutting down i am saving the customerId into the file and whenever services is up then i reading entry from the file so
this way i am making sure that customerId should not again start from initial value and new customer will get oldValue +1.

What if you have two Bank instances in different threads? Can you get duplicate IDs in those instances?

i am using Atomic Integer which is thread safe so i think this will handle the case as well and no duplicate. Though i have not tested in multi threaded environment.

Consider a shutdown hook on the runtime which does one of these things

I have added shutdown Hook up in my BankStarter Class. As of now i am just storing customerId into the file. Once i will implements other i will
add them to. I defined a inner class in my Bank class for the same.

Looks good
Remember you are adding the shutdown hook to the Runtime, not a particular class. Also remember the Runnable must not do anything taking a long time because the lifetime of the JVM after the shutdown hook is activated is very short.

Now let's see you write that in Java8 notation without defining a Runnable class!

Now let's see you write that in Java8 notation without defining a Runnable class!

Do you mean using Lambda expression? I do not have any idea but let me read about it.

Search my posts for the last week and look out for a spactecular misspelling.

Try here for the wrong spelling.

Try here for the wrong spelling.

SingUtilities or SwingUtilities... Syntax are quite different for JRE-8, i need to read whole story about expressions... Pardon me for later replying as i was very busy from last 2 -3 days due
to family issue.. I will finish my work by today evening and start again concentrate on it....

Now let's see you write that in Java8 notation without defining a Runnable class!

Finally, i am able to get it. It looks better ad short. Is it what you referring to?




Also i have removed local inner class from the Bank and instead defined a static method to store data.

Agree it seems better without the inner class. But why is the store data method static?

Because i think it is independent to the instances though i agree there will be only one instance of the Bank class. So i thought both are equal but i think you are pointing
to possible risk to access the method directly from the Class?

… and where will the data come from, if not from the instance?

got your point. Thanks for clearing it.

You're welcome

So my customer class is now looks like this. Is this correct now?

Looks all right.

But…
Is LocalDate immutable?
Why are you using a method rather than the field in equals? Why are you using else after a return? I prefer only to use return once per method so I would reduce that method to
return other == this || other != null && other.getClass().equals(this.getClass()) && ((Customer)other).id == id;
You do have to keep the terms in that order and use || && not | & to avoid a null exception.
Do you really want to reset names with set methods? What validation of names are you doing in those set methods?

Search these fora to find out about the controversy about whether you should or shouldn't use getClass in an equals method.

Is LocalDate immutable?

Yes it is..

Do you really want to reset names with set methods?

No. Removed now.

whether you should or shouldn't use getClass in an equals method.

Yes we should use getClass instead of instanceOf because instanceOf will not return false in case if argument is subclass of the original class but getClass will
return false. I refer this link http://www.javaranch.com/journal/2002/10/equalhash.html

What validation of names are you doing in those set methods?

No validation in set method but i think validation in terms of only alphabets and single whitespace allowed in name should be done in Bank class which accepting string from the utility class.

Why are you using a method rather than the field in equals? Why are you using else after a return?

for better readability. Thanks for the suggestion, i almost forgot this way to get fields from the object. I have update the code as suggested as well.

Tushar Goel wrote:

Is LocalDate immutable?

Yes it is.. . . .

You may have checked that before, but I couldn't remember, so I asked it again.

Coming on nicely

It is still worth reading the controversy about the equals method. It means that once you have an overridden equals method you should not add any fields in subclasses.