Are Empty Interfaces Code Smell

Are empty interfaces code smell?

Although it seems there exists a design pattern (a lot have mentioned "marker interface" now) for that use case, i believe that the usage of such a practice is an indication of a code smell (most of the time at least).

As @V4Vendetta posted, there is a static analysis rule that targets this:
http://msdn.microsoft.com/en-us/library/ms182128(v=VS.100).aspx

If your design includes empty interfaces that types are expected to implement, you are probably using an interface as a marker or a way to identify a group of types. If this identification will occur at run time, the correct way to accomplish this is to use a custom attribute. Use the presence or absence of the attribute, or the properties of the attribute, to identify the target types. If the identification must occur at compile time, then it is acceptable to use an empty interface.

This is the quoted MSDN recommendation:

Remove the interface or add members to it. If the empty interface is being used to label a set of types, replace the interface with a custom attribute.

This also reflects the Critique section of the already posted wikipedia link.

A major problem with marker interfaces is that an interface defines a contract for implementing classes, and that contract is inherited by all subclasses. This means that you cannot "unimplement" a marker. In the example given, if you create a subclass that you do not want to serialize (perhaps because it depends on transient state), you must resort to explicitly throwing NotSerializableException (per ObjectOutputStream docs).

What is an empty interface used for?

This is called a "marker interface." Sometimes they're used to indicate that a class is meant for a certain purpose. It's not a desirable practice.

While I have used marker interfaces, here's an illustration of the problem they create. Suppose I have a List<ICube>. Perhaps I receive it as a method argument.

public interface ICube {} // It's empty!

public void DoSomethingWithTheseCubes(List<ICube> cubes)
{
    foreach(var cube in cubes)
    {
        // what do I do with this cube?
    }
}

You can see where I get stuck. ICube is just a marker interface, so it has no methods or properties of its own. I can't do anything with it. That's likely to result in me casting each cube to some other type so I can do something with it.

public void DoSomethingWithTheseCubes(List<ICube> cubes)
{
    foreach(var cube in cubes)
    {
        (SomeOtherType)cube.DoSomething();
    }
}

But if I cast it I'm inviting a runtime error because I may not know for sure what the actual runtime type of each object is. If I know what the runtime type is, then I just should do this:

public void DoSomethingWithTheseCubes(List<SomeOtherType> things)
{
    foreach(var thing in things)
    {
        thing.DoSomething();
    }
}

We're not absolutely certain to have that problem, but using a marker interface invites it. It's using an interface for something other than its intended purpose. It's more like an attribute or even a comment.

An interface has two uses that work together: First, it describes the members that a class implements. Second, it allows us to cast a class that implements an interface as that interface. Marker interfaces do neither. They allow us to cast an object as a type with no members. That's useless at best, and at worst it's harmful because it leads to even more questionable casting.

Interfaces, Classes and Code Smell

You could create a interface that extends both Weapon and Reloadable.

public interface WeaponReloadable extends Weapon,Reloadable {
   ...
}

And having this implementation :

public final class MyWeaponReloadable implements  WeaponReloadable  {
   ...
}

In this way you could create an instance of MyReloadableWeapon declared with the ReloadableWeapon interface and pass it in methods with Reloadable or Weapon parameters :

WeaponReloadable weaponReloadable = new MyWeaponReloadable();
reloadGameWeapon(weaponReloadable);
attackGameWeapon(weaponReloadable);

Is it code-smelly to have empty classes in the middle of a class hierarchy?

In general, empty classes are a code smell.

I agree your isLeaf or isBranch methods are a correct alternative.
They add information about the objects , which is helpful.
(This is because, on the super class, you can't express that subclasses are "either leaf or branch").

---

The two methods with opposite results might also be considered as code duplication.

You could use only one... But I would recommend return an enumerated value LEAF or BRANCH.

---

Related Topics