Is it possible to avoid using type checking in this example?

Question

Sorry for the poor title, can't think of a succinct way of putting this..

I'm thinking of having a list of objects that will all be of a specific interface. Each of these objects may then implement further interfaces, but there is no guarantee which object will implement which. However, in a single loop, I wish to be able to call the methods of whatever their further sub-type may be.

ie, 3 interfaces:

public interface IAnimal { ... }
public interface IEggLayer { public Egg layEgg(); }
public interface IMammal { public void sweat(); }

this would then be stored as

private List<IAnimal> animals= new ArrayList<IAnimal>();

so, instances added to the list could possibly also be of type IEggLayer or IMammal, which have completely unrelated methods.

My initial instinct would be to then do

for(IAnimal animal : animals) {
  if(animal instanceof IEggLayer) {
    egg = ((IEggLayer)animal).layEgg();
  }
  if(animal instance of IMammal) {
    ((IMammal)animal).sweat();
  }
}

But I have always been told that type checking is a sign that the code should really be refactored.

Since it could be possible for a single object to do both [platypus, for example], meaning that a single doFunction() would not be suitable here, is it possible to avoid using type checking in this case, or is this an instance where type checking is classed as acceptable?
Is there possibly a design pattern catered to this?

I apologise for the contrived example as well...
[ignore any syntax errors, please - it's only intended to be java-like pseudocode]

Edit: added lvalue to the EggLayer use, to show that sometimes the return type is important

Answer 1

Clearly your IAnimal interface (or some extension thereof) needs a callAllMethods method that each implementer of the interface can code to polymorphically perform this task -- seems the only OO-sound approach!

Answer 2

in C#, you should be able to do this transparently.

foreach(IEggLayer egglayer in animals) {
    egglayer.layEgg();
}

foreach(IMammal mammal in animals) {
    mammal.sweat();
}

Answer 3

Why not have methods added to isAnimal:

public interface IAnimal {
   bool isEggLayer();
   bool isMammal();
}

Then you can loop through and just query this boolean each time.

Update: If this was drawing an animal, then having a class that is completely enclosed is reasonable, you just call drawVehicle and it draws a corvette, cessna, motorcycle, whatever.

But, this seems to have a non-OOP architecture, so if the architecture of the application can't change then, since my original answer isn't well received, then it would seem that AOP would be the best choice.

If you put an annotation on each class, you can have

@IsEggLayer
@IsMammal
public class Platypus() implements EggLayer, Mammal {
...
}

This would then enable you to create aspects that pull out all the egglayers and do whatever operations need to be done.

You can also inject into the animal interfaces any additional classes to get this idea to work.

I will need to think about where to go from here, but I have a feeling this may be the best solution, if a redesign can't be done.

Answer 4

I think the way to think about this question is: What is the loop doing? The loop has a purpose and is trying to do something with those objects. That something can have a method on the IAnimal interface, and the implementations can sweat or lay eggs as needed.

In terms of your issue with the return value, you will be returning null, nothing you can do about that if you share the methods. It is not worth casting within a loop to avoid an extra return null; to satisfy the compiler. You can, however, make it more explicit using generics:

  public interface IAnimal<R> {

         public R generalMethod();

  }

  public interface IEggLayer extends IAnimal<Egg> {

         public Egg generalMethod(); //not necessary, but the point is it works.

  }

  public interface IMammal extends IAnimal<Void> {

        public Void generalMethod();

  }

From your comment where you care about the return type, you can get the return type and dispatch it to a factory method which examines the type and returns something generic that is sublcassed to the specific type and act on that.

Answer 5

But I have always been told that type checking is a sign that the code should really be refactored.

It is a sign that either class hierarchy or the code that uses it may need to be refactored or restructured. But often there will be no refactoring / restructuring that avoids the problem.

In this case, where you have methods that apply only to specific subtypes, the most promising refactor would be to have separate lists for the animals that are egg layers and the animals that sweat.

But if you cannot do that, you will need to do some type checking. Even the isEggLayer() / isMammal() involves a type check; e.g.

if (x.isEggLayer()) {
    ((IEggLayer) x).layEgg();  // type cast is required.
}

I suppose that you could hide the type check via an asEggLayer() method; e.g.

public IEggLayer asEggLayer() {
    return ((IEggLayer) this);
}

or

// Not recommended ...
public IEggLayer asEggLayer() {
    return (this instanceof IEggLayer) ? ((IEggLayer) this) : null;
}

But there is always a typecheck happening, and the possibility that it will fail. Furthermore, all of these attempts to hide the type checking entail adding "knowledge" of the subtypes to the supertype interface, which means that it needs to be changed as new subtypes are added.

Answer 6

There are many ways of going about this. Exaclty which is most appropriate depends upon your context. I am going to suggest introducing an additional layer of indirection. This solves most problems. I prefer designs which avoid multiple inheritance of interface (if I were president of a language, I would forbid it).

I don't think layEggsOrSweatOrDoBothIfYoureWeirdOrNoneIfYouCant() is a great method to polute Animal with. So instead of adding each Animal directly to animals, wrap each in an individual wrapper. I say "wrapper" as a generic name - the random operation we are trying to perform doesn't make any sense.

private final List<AnimalWrapper> animals =
    new ArrayList<AnimalWrapper>();

public void doStuff() {
    for (AnimalWrapper animal : animals) {
        animal.doStuff();
    }
}

Then we need some way of adding the wrappers. Something like:

public void addPlatypus(final Platypus platypus) {
    animals.add(new AnimalWrapper() { public void doYourStuff() {
        platypus.sweat();
        platypus.layEgg();
    }});
}

If you try to write these wrappers without enough context you get into trouble. These require that the correct one is selected at call site. It could be done by overloading, but that has dangers.

/*** Poor context -> trouble ***/

public void addNormalMamal(final Mamal mamal) {
    animals.add(new AnimalWrapper() { public void doYourStuff() {
        mamal.sweat();
    }});
}
public void addNormalEggLayer(final EggLayer eggLayer) {
    animals.add(new AnimalWrapper() { public void doYourStuff() {
        eggLayer.layEgg();
    }});
}
public <T extends Mamal & EggLayer> void addMamalEggLayer(final T animal) {
    animals.add(new AnimalWrapper() { public void doYourStuff() {
        animal.sweat();
        animal.layEgg();
    }});
}