Popular belief is that inheritance is always a good thing. However when used inappropriately, it can lead to code with bugs. The problems have been well documented in several books and articles. Yet very often we come across code with a complex inheritance hierarchy that looks cute, but is difficult to use and more often than not broken. Below I recap a few key points to remember when using inheritance in Java.
Consider the class MemberStore which is used to persist Members to a database
public class MemberStore { public void insert(Member m) { // insert into database } public void insertAll(List<member> c) { // insert all members into the database // by calling the insert(m) for each Member Iterator i = c.iterator() ; while(i.hasNext()) { insert((Member)c.next()) ; } } }Let us say there is a need for a class that not only stores Members, but also keeps count of recently stored members. The wrong way to do it would be to
extend the above class
public class MemberStoreWithCounter extends MemberStore { private int counter = 0 ; public void insert(Member m) { counter++ ; super.insert(m) ; } public void insertAll(Collection<member> c) { counter = counter + c.size() ; super.insertAll(c) ; } public int getCount() { return counter ; } }
Let us try to use the new class
List<Member> list = new ArrayList<Member>() ; list.add(new Member(......)) ; List.add(new Member(......)) ; MemberStorewithCounter m = new .... ; m.insertAll(list) ; System.out.println(a.getCount());What do you suppose the last line prints ? You were probably expecting 2. But it will print 4. The problem is that the insertAll method is implemented by calling the insert method. So the counter is incremented twice for each Member. This is the classic wrong use of inheritance.
The problem is that inheritance breaks encapsulation. Inheritance requires the programmer implementing the subclass class to know the implementation details of the super class. So generally it is safe to use inheritance only when the subclass and the superclass are under the ownership of the same programmer or team of programmers.
If you are designing your class so that other programmers may extend it, then it is preferable that the implemented methods of your class not call other overridable methods as was done by insertAll. If they do, then the javadocs must clearly document it. In the above example, the javadocs should say that insertAll calls insert. But this is not sufficient.
The javadocs for methods that can be safely overridden should say under what conditions and for what behavior the method should be overridden.
Classes that are not meant to be extended and methods that should not be overridden should be marked final so that programmers cannot extend them.
Constructors should never invoke overridable methods.
Lastly, in Java inheritance implies an "generalization/specialization" or "is a" relationship. Even if you do not intend to specialize, if you add a method to the subclass with the same signature of a method in the subclass, the method in the subclass overrides the method in the superclass. If there is no "is a" relationship, it is never a good idea to use inheritance.
The better way to add a counter to MemberStore is to write a wrapper around MemberStore that has MemberStore as a private member and delegates to it. This is otherwise known as composition.
public class BetterMemberStoreWithCounter { private MemberStore ms = new MemberStore() ; private int count = 0 ; public insert(Member m) { ms.insert(m) ; count++ ; } public insertAll(Collection<member> c) { count = count.size() ; ms.insertAll(c) ; } }
Nice writup Manoj. Its worth reading it. though I am not big fan of inheritance and always try to replace Inheritance with composition whenever possible because of runtime flexibility it provides. look forward for some more nice writeup keep it up.
ReplyDeleteThanks
Javin
How classpath works in Java
Hi Manoj,
ReplyDeleteI agree that improper use of Inheritance leads to bad design and buggy code. But in your above sample code the problem is not with inheritance. It is just a buggy subclassing of parent class.
The original MemberStore is doing its job correctly. Now in the subclass, the bug is introduced by setting counter = counter + c.size() ; in subclass insertAll().
If you comment it out, it will work fine.
There are many other design level problems if we use inheritance excessively. Using Composition is a better way as you explained above.
Thanks,
Siva
@javin, thanks
ReplyDeleteI agree with siva, there is no problem with MemberStore, when you inherit from a class you MUST know HOW it works.
ReplyDeleteWhen using inheritance just the insert method needs to be overloaded, in the composition example your are fully repeating all methods just for a very simple new feature (count).