Sunday, October 28, 2007

The risk of invoking public methods in Java

In the past few posts we have discussed how it can be dangerous to extend a class that we do not control, if it has not been designed for subclassing. We understood this concept with a specific example from Josh Bloch's book: Effective Java. In the example we subclassed the HashSet class in the JDK.

Without repeating the material, I will simply reiterate what I said in my previous post:

The real problem was the class AbstractCollection which was not designed properly for inheritance. It is a non-final class with a public method addAll(Collection c) which invokes another non-final public method add(Object o).

Here is a possible implementation of the HashSet class that does not break the DRY principle and is also safe for extending...

Saturday, October 20, 2007

Never invoke a public method from another public method

In the previous post we saw how subclassing from a class not designed for inheritance, can be dangerous. Using composition instead of inheritance would have solved the problem. But what was the crux of the problem? Could such a trap have been avoided? Could AbstractCollection have been implemented so that such a situation would not have arisen in the first place?

The real problem was the class AbstractCollection which was not designed properly for inheritance. It is a non-final class with a public method addAll(Collection c) which invokes another non-final public method add(Object o).

How should AbstractCollection have implemented add(Object o) and addAll(Collection c) to prevent such a problem?

I do not know if this problem has been fixed in OpenJDK. They may have not been able to do so to maintain backwards compatibility. You can check out the sources from their website.

In the next post we will discuss an idiom for preventing such a situation. 

Discuss this post in the learning forum.



Note: This text was originally posted on my earlier blog at http://www.adaptivelearningonline.net

Tuesday, October 16, 2007

A Java example showing the dangers of implementation inheritance

I ended my previous post with a paragraph from Josh Bloch's book, Effective Java. In his book, Josh explains why inheriting classes from a different package (especially inheriting from a class that may not be designed and documented for inheritance) can be dangerous. Below you will find Josh's example (from Effective Java) that explains the concept. Many many thanks to Josh Bloch for graciously giving me permission to reproduce the example here :-)

The class InstrumentedHashSet is a subclass of HashSet, that maintains a count of the number of objects added. It overrides the add(Object o) and addAll(Collection c) methods from HashSet to maintain a count of objects added to it. These methods increment the count and then delegate responsibility for adding the element to HashSet. The number of objects added can be obtained from the method getAddCount().

This seems like a reasonable way to count objects added to the Set. Unfortunately it's not. But first let's have a look at the code of InstrumentedHashSet.

 InstrumentedHashSet.JPG

 

Below is a test case for the addAll(Collection c) method of InstrumentedHashSet. By the way, this test was written by me, so if there is anything wrong with it, it's entirely my fault :-)

 

InstrumentedHashSetTest.JPG 

 

Now let us run the unit test.

InstrumentedHashSetTestReport.JPG 

Oops :-( we expected a count of 3, but looks like InstrumentedHashSet is reporting 6. Now, how did that happen???

Look again at InstrumentedHashSet. The method addAll(Collection c) calls super.addAll(c) after incrementing the count. This ends up invoking a method implemented in AbstractCollection in the JDK (ver 1.6). It has implemented addAll(Collection c) to iterate through the given Collection and invoke add(Object o) for every element in the Collection. Since we have overriden add(Object o), it is our implementation that get's called. The count is added for every element of the Collection in addAll(Collection c) as well as in add(Object o), resulting in the count increasing by 2 for every object. That's the reason we got a count of 6 instead of 3.

So to summarize. Be very careful when you subclass a class that is not in your control. It might be safer to use composition that inheritance in such situations. It is also equally important to remember that if you create a non-final class with public methods, please design it carefuly to ensure that it does not break a subclass that may extend from it. If there are any caveats, be sure to document them clearly.



Note: This text was originally posted on my earlier blog at http://www.adaptivelearningonline.net
Here are the comments from the original post

-----
COMMENT:
AUTHOR: Sanket Daru
URL:
DATE: 10/20/2007 11:40:37 AM
This was a serious eye opener.

Learnt a very good OOP lesson.