Are empty abstract classes a bad practice, and why

Question

We have several empty abstract class in our codebase. I find that ugly. But besides this very stupid reason (ugliness), should I refactor it (into empty interface e.g.) ?

Otherwise, the code is robust and well tested. So if it's only for a "aesthetic" reason, I will pass and let the empty abstract classes remain.

What do you think?

EDIT :

1) By "empty abstract class", I mean something like :

public abstract class EmptyAbstractClass {}

2) The reason for the "emptiness" : Hibernate. I don't master this persistence framework at all. I just understand that an interface cannot be mapped to a table, and for this technical reason, a class has been preferred to an interface.

Answer 1

According to object oriented programming theory the main purpose for inheritance is polymorphism, code reuse and encapsulation. An empty abstract class (and when i say this i mean truly empty, no constructors, no methods, no properties. nothing!) does not achieve any of the three goals hoped by this programming technique. It is the equivalent to if(true){...}. changing it to an interface does not really makes it any better.

If you want to refactor the code i would advise you to think in the direction opposite to the one you are thinking, what i mean by this is: try to abstract properties, methods and constructors from all classes that share a abstract parent class.

This is hard work with little reward in the short term but it increases the maintainability of the code dramatically since a core logic change would have to be done only once. I suspect the reason for using those empty abstract classes is to identify a group of classes that must share something in common otherwise what would be the difference between Object and the abstract class

Answer 2

The question to ask is: "What do I want to do with this code that I can't do, or find hard to do, because these empty abstract classes are there?" If the answer is 'nothing', you should leave them alone. If the answer is "something", it may well be worthwhile to remove them - but if you can, speak to the people who created them first, just to make sure there isn't some subtle purpose to them. For example, perhaps your code uses reflection to find all instances of a particular ABC and do special things to them, in which case your refactoring could break the code in subtle ways.

Answer 3

If you have the following pattern you will find it to be the most flexible:

interface Fooable
{
   void foo();
   void bar();
}

abstract class AbstractFoo
    implements Fooable
{
}

class Foo
    extends AbstractFoo
{
   public void foo()
   {
   }

   public void bar()
   {
   }
}

This way you can always pass by the interface but if you later find that you have code that can be common you can put it in the abstract class without having to make changes to all of the classes.

Unless you have a good reason for not doing that pattern (and I suspect you don't in this case) I would suggest using it. This can wind up with empty abstract classes but I think it is ok (a little odd, but ok).

If there truly are no methods in the interface or only one then I would skip the abstract class.

From a compiler/functional point of view there is no real difference between an interface and an abstract class where all method are abstract. The interface will be more flexible than the abstract class, and the interface + abstract class will be the most flexible of all.

If it were my code I'd make the change to them being interfaces...

Answer 4

Empty abstract classes don't make any sense to me, abstract classes should be used to inherit some behavior. So, I tend to agree, it's a pretty ugly design and a very bad use of abstract classes, marker interfaces should be preferred. So you have two options:

1. Wait till you need to extend another class for a real need and get annoyed by the abstract class to replace it by an interface.
2. Don't wait and fix the design.

In this particular situation, it is true that the actual design doesn't really hurt, for now, so you can live with it. However, I think replacing these abstract classes is a pretty easy refactoring (make them interfaces and replaces extends with implements where you get compilation errors) and I really can't see what could get broken so I would go for it.

Personally, and people may of course disagree, I don't like being too defensive. With rules like if it's ain't broke, don't fix it, you'll never refactor anything ("my tests passes, why should I refactor?"). Freezing the code is definitely not the right solution, testing aggressively is the right solution.

Answer 5

While I don't get what will be in the table that the empty class is mapped to, if the class serves some purpose, well, keep it, until you have an opportunity to refactor some.

What I would definitely do is: write a comment about why this class exists in the file. One big reason for clean and "beautiful" code is to not make other developers think. A comment can help with that, even if the code is not as pretty as it could be.

Answer 6

Interfaces are preferable in this case because it makes your code more robust for maintenance. That is, you can only extend a single class but you may implement many interfaces.

If there is absolutely no direct effect right now I would not touch it. If the maintenance event turns up that requires you to make a change then I would refactor it since I am already in the code.

In other words if it ain't broke don't fix it.