14

I'm new to Java and struggling with a design problem. I know use of instanceof may indicate a design flaw and I understand the often given Animal/Dog/Cat classes as example, replacing bark() and meow() with makenoise() etc.

My question is, what is a sensible design if I need to call methods which do not have a corresponding method depending on the type of subclass? For example, what if I want to call a new method biteleash() if the class is a Dog but do nothing at all if it's a Cat?

I did consider having biteleash() in Animal which does nothing, and overriding it in Dog, but there are methods many like this so it seems a clunky solution. In a similar vein, what if the caller needs to do something different depending on which subclass it has hold of, eg. terminate if subclass is a Cat? Is instanceof acceptable here, or is there a better way?

public class Animal {

    String name;

    public Animal(String name) {
        this.name = name;
    }

    public String getName() {
        return name;
    }

    public void makeNoise() {
        System.out.println("Some noise for a generic animal!");
    }

}

public class Cat extends Animal {

    public Cat(String name) {
        super(name);
    }

    @Override
    public void makeNoise() {
        System.out.println("Meow");
    }
}

public class Dog extends Animal {

    public Dog(String name) {
        super(name);
    }

    @Override
    public void makeNoise() {
        System.out.println("Woof");
    }

    public void biteLeash() {
        System.out.println("Leash snapped!");
    }
}

import java.util.Random;

public class CodeExample {


    public static void main(String[] args) {

        Animal animal = getSomeAnimal();
        System.out.println("My pet is called " + animal.getName());
        animal.makeNoise();

        if (animal instanceof Dog) {
            Dog dog = (Dog) animal;
            dog.biteLeash();
            // do lots of other things because animal is a dog
            // eg. sign up for puppy training lessons
        }
    }

    private static Animal getSomeAnimal() {
        Animal animal;
        Random randomGenerator = new Random();
        int randomInt = randomGenerator.nextInt(100);      
        if (randomInt < 50) {
            animal = new Dog("Rover");
        }
        else {
            animal = new Cat("Tiddles");
        }
        return animal;
    }
}
Amal Dev
  • 1,938
  • 1
  • 14
  • 26
scatter
  • 157
  • 4
  • 1
    I think you could have an `enum` of `AnimalType` and have a `AnimalType` member in `Animal` and check that instead of `instanceof`. – M. Shaw May 15 '15 at 09:21
  • 6
    The animal analogy is obfuscating you. Why would you invoke `biteleash` on `Animal`? What is the use case? Do you want for all `Animal` objects to do animal specific behaviour at this point? Then make `animalSpecificBehaviour` and define it for `Dog` to invoke `biteleash`. The question of architecture has to be after the analysis of how and under what circumstances you would invoke it. – Amadan May 15 '15 at 09:22

9 Answers9

10

Composition will help you here, and is idiomatic in Java.

Design an interface called, say, Leashable. This is implemented by a Dog, but not a Cat.

Rather than using instanceof, you can attempt a reference cast to Leashable to see if it's implemented by your particular object.

In my opinion, you should continue in a similar vein: Build a NoisyAnimal interface too. Perhaps even just Noisy as why should noisiness be pertinent to only animals? Implementing that for a Parrot, say, will have technical challenges beyond a Cat or Dog. Good maintainable programs isolate areas of complexity and composition helps you achieve that.

Bathsheba
  • 231,907
  • 34
  • 361
  • 483
  • 3
    It is easier to check `instanceof` then trying casting to interface and support catching ClassCastExceptions. – FazoM May 15 '15 at 09:22
  • Indeed but composition will *scale* better. – Bathsheba May 15 '15 at 09:23
  • what is a reference cast to an interface? – tim May 15 '15 at 09:26
  • 4
    @tim I think `try{Leashable leDog = (Leashable) aniDog} catch(..){...}`, in which case `instanceof` is definitely a better option. You still need to make an explicit cast, but expection handling is an antipattern – John Dvorak May 15 '15 at 10:26
  • @JanDvorak So, if instanceof is better than this patter, why is this the top answer? – tim May 15 '15 at 23:16
5

You shouldn't use concrete classes. Instanceof itself isnt a problem. It exists for a reason. You should use interfaces for loose coupling i.e. your code shouldnt be dependent on concrete class implementations. I suggest you using interfaces wherever possible (i.e. IAnimal instead of Animal)

Instead of checking for Dog, you should use an interface like ILeashable (yeah a bit ridiculous for a name lol), then:

public interface ILeashable {
   //add other methods which is connected to being on a leash
   void biteLeash();
}

class Dog implements ILeashable {...}

Also there is no one way of doing this, there are certain patterns i.e. decorators or Dependency Inversion which might help you in this case.

breakline
  • 5,776
  • 8
  • 45
  • 84
1

Just so you know, this issue you're having is not something you'll generally be facing in the real world. If you have to have implementation-specific logic on some class that implements an interface or an abstract base class, it will usually be because at some higher level you need to get a derived property. This psuedocode to illustrate:

interface ISellable {
     decimal getPrice();
}
class CaseItem : ISellable {
    int numItemsInCase;
    decimal pricePerUnit;
    decimal getPrice() {
         return numItemsInCase*pricePerUnit;
    }
}
class IndividualItem : ISellable{
    decimal pricePerUnit;
    decimal getPrice() {
         return pricePerUnit;
    }
}
main() {
    aCaseItem = new CaseItem { pricePerUnit = 2, numItemsInCase=5 }; //getPrice() returns 10
    anIndividualItem = new IndividualItem { pricePerUnit = 5 }; //getPrice() returns 5

    List<ISellable> order = new List<ISellable>();
    order.Add(aCaseItem);
    order.Add(anIndividualItem);

    print getOrderTotal(order);
}
function getOrderTotal(List<ISellable> sellableItems) {
    return sellableItems.Sum(i => i.getPrice());
}

Notice that I am using the interface to abstract away the concept of an item's price, but when I'm actually in the main method, I can easily create instances of the specific type in order to control the behaviors of the two classes.

However, when I need to get the price, I'm referencing the items as a list of ISellable, which only exposes their "getPrice()" method for my convenience.

Personally, I always believed the animal methaphor to be severely lacking. It doesn't explain this concept in a way that makes sense, and it doesn't clue you in on how to use it in the real world.

C Bauer
  • 5,003
  • 4
  • 33
  • 62
1

Think of it this way: What event causes the Dog to bite its leash? Or differently speaking, what is your motivation to make it perform this action?

In your example there is actually none. It just so happens that in your main method you decided to put in a check and if the animal you randomly created is a Dog, you make it do certain dog things. That's not how code in the real world works.

When you write code, you want to solve some problem. To stick to the animals example, let's pretend you write a game where you have pets that react to certain events like turning on the vacuum or getting a treat. From this sentence alone we can create a reasonable class hierarchy:

interface Animal {

    void reactToVacuum();
    void receiveTreat();

}

class Dog implements Animal {

    public void biteLeash() {
        System.out.println("Leash snapped!");
    }

    public void wiggleTail() {
        System.out.println("Tail is wiggling!");
    }

    @Override
    public void reactToVacuum() {
        biteLeash();
    }

    @Override
    public void receiveTreat() {
        wiggleTail();
    }
}

As you can see, the leash biting happens in response to an event, namely turning on the vacuum.

For a more realistic example, take Android's view hierarchy. A View is the base class for every control on the screen, e.g. a Button, an EditText and so on. View.onDraw() is defined for every view but depending on what View you have, something different will happen. EditText for example will do something like drawCursor() and drawText().

As you can see, the answer to the question "What event causes the EditText to draw the cursor" is "It needs to be drawn on the screen". No instanceof or condition checking necessary.

Kirill Rakhman
  • 42,195
  • 18
  • 124
  • 148
0

As in your example - some subclasses can have different methods.
This is typical scenario where you have to use instanceof.

FazoM
  • 4,777
  • 6
  • 43
  • 61
0

If your subclasses are fixed then you can use animalType to avoid instanceof.

I prefer that u can use interface to check whether the feature is available with the subclass, but it is not feasible for large number of subclass dependent methods. ( solution given by user2710256).

Enum Type {
        DOG,CAT ;
    }

    public abstract class Animal {

        String name;

        Type type;

        public Animal(String name) {
            this.name = name;
        }

        public String getName() {
            return name;
        }

        public void makeNoise() {
            System.out.println("Some noise for a generic animal!");
        }

        abstract Type getType();

    }

    public class Cat extends Animal {

        public Cat(String name) {
            super(name);
        }

        @Override
        public void makeNoise() {
            System.out.println("Meow");
        }

        public Type getType() {
            return Type.CAT;
        }
    }

    public class Dog extends Animal {


        public Dog(String name) {
            super(name);
        }

        @Override
        public void makeNoise() {
            System.out.println("Woof");
        }

        public void biteLeash() {
            System.out.println("Leash snapped!");
        }

        public Type getType(){
            return Type.DOG;
        }
    }

    import java.util.Random;

    public class CodeExample {


        public static void main(String[] args) {

            Animal animal = getSomeAnimal();
            System.out.println("My pet is called " + animal.getName());
            animal.makeNoise();

            switch(animal.getType())
            {
               case DOG:  
                        {
                        Dog dog = (Dog) animal;
                        dog.biteLeash();
                        // do lots of other things because animal is a dog
                        // eg. sign up for puppy training lessons
                        }

                case CAT:
                        {
                            // do cat stuff
                        }
                default: 
                        throw new Exception("Invalid Animal");
            }
        }

        private static Animal getSomeAnimal() {
            Animal animal;
            Random randomGenerator = new Random();
            int randomInt = randomGenerator.nextInt(100);      
            if (randomInt < 50) {
                animal = new Dog("Rover");
            }
            else {
                animal = new Cat("Tiddles");
            }
            return animal;
        }
    }
Sagar Gandhi
  • 925
  • 6
  • 20
0

Of course having biteleash() in a Cat doesn't make sense, for you don't walk cats on a leash. So Animal shouldn't have biteleash(). Maybe you should put a method like playWith() in an Animal, in which Dogs would biteleash(). What I'm trying to say is you probably need to be more general. You shouldn't care if it is a dog to manually call biteleash(). Adding a Ferret to your Zoo would require adding the possibility that the animal is a Ferret, since they can also biteleash().

And maybe a method shouldTerminate(), which returns true in Cats. You will probably want to terminate also on a Tortoise in a while.

Generally, People consider it bad practice, if adding a new subclass would require changes in code using this class.

ki92
  • 320
  • 2
  • 7
0

If I may, this is a highly impractical way to do this, I'm not sure what you are after but instanceof is going to be tedious, what you end up with is high coupling which is a no go when doing OOD. Here is what I'd do:

First, get rid of class Animal and make it an interface as following

public interface Animal {

    public String getName();

    public void makeNoise();

    public void performAggression();
}

then in dog:

public class Dog implements Animal {

    private String name;

    public Dog(String name) {
        this.name = name;
    }

    @Override
    public String getName() {
        return name;
    }

    @Override
    public void makeNoise() {
        System.out.println("Woof");
    }

    @Override
    public void performAggression(){
        biteLeash();
    }

    private void biteLeash() {
        System.out.println("Leash snapped!");
    }
}

lastly, the cat:

public class Cat implements Animal {

    private String name;

    public cat(String name) {
        this.name = name;
    }

    @Override
    public String getName() {
        return name;
    }

    @Override
    public void makeNoise() {
        System.out.println("Meow");
    }

    @Override
    public void performAggression(){
        hiss();
    }

    private void hiss() {
        System.out.println("Hisssssss");
    }
}

This way you can in you main class just do as following:

public class test{

    public test(){
        dog = new Dog("King");
        cat = new Cat("MissPrincess");
    }

    public void performAggression(Animal animal){
        animal.performAggression();
    }

The bonus you get is, you can pass whatever class you want to a method, as long as they implements the same interface, as shown in the test class above in the method performAggression(Animal animal)

All the test class needs to know is those 3 methods, everything else can be done internally in the respective classes without test class needs to know anything about it, hence the "private" visibility on the biteLeash() and hiss() methods.

You end up with a very low coupling and easy to edit later on.

By doing this you effectively also achieve high cohesion because you don't have to get involved in mile long if/ifelse/ifelse...(so on) to determine what kind of class you are dealing with.

Radiodef
  • 37,180
  • 14
  • 90
  • 125
Simon Jensen
  • 488
  • 1
  • 3
  • 19
  • I apologize for any spelling mistakes I may have bade and smaller mistakes, thanks Radiofed for correcting these. However, as it implements an interface, and does not expend a super-class, it does not need override tags on the methods. – Simon Jensen May 15 '15 at 17:29
0

Operator instanceof tends to scale well only when applied to interfaces. The reason is that an interface partitions your domain in two: some objects are instances of classes which implement the interface, and the others are not. There is very likely no way that introducing a new class will break your current algorithms that rely on if (x instanceof ISomething) because the new class will either implement ISomething or not. If it does, the if body should cast x to ISomething and make good use of it; if it doesn't then the body of the if is probably of no interest to x.

This is different from the approach in which instanceof is applied to classes. For instance, seals do bark but they are not dogs: you cannot inherit Seal from Dog or viceversa. Hence, as you hinted, you are aware of the fact that if you use instanceof to check whether you can make x bark, you may end up with abominations like this:

if (x instanceof Dog) {
    ((Dog)x).bark();
}
else if (x instanceof Seal) {
    ((Seal)x).bark();
}

The solution is to have an interface ICanBark and test for it:

if (x instanceof ICanBark) {
    ((ICanBark)x).bark();
}

This scales well if you add more animals, whether they can bark or not. But it doesn't scale well if you want to add more noises, because you may be tempted to introduce more interfaces like ICanMeow and the like.

The solution is avoid to be in that situation; ICanBark and ICanMeow are obviously the same behavior IMakeNoise so that there is a single interface implemented by Dog, Cat and Seal.

I'm not a fan of the animal/dog/cat/noise example in the context of polymorphism. Instead I'll show you a real world example of a component system I was working on some time ago. In a component system, components are installed on entities to give them additional behavior. For instance, a monster in a videogame is an entity with some component installed (this stuff is inspired from the Unity 5 engine):

  1. IAudioSource
  2. IRenderer
  3. IMovementAI
  4. ILiving

The four classes that may implement that behaviours may well be something along the lines of:

  1. ScaryGruntingSound
  2. MeshOpenGLRender
  3. StupidQuadrupedAI
  4. Armorless

If you have a collection of entities then it is then totally acceptable (and it is going to scale well) if you instanceof for the interfaces:

for (Entity e : entities) {
    for (Component c : e.getComponents()) {
        if (c instanceof IAudioSource) {
            ((IAudioSource)c).play();
        }

        ...

        if (c instanceof IUserInput) {
            ((IUserInput)c).poll();
        }
    }
}

Since monsters are not controlled by the player, c instanceof IUserInput fails and everybody is happy. The reason why this use of instanceof rocks and the ICanBark/ICanMeow sucks is because IAudioSource/IRenderer/IMovementAI/ILiving are four totally unrelated aspects of what it means to be a monster, while ICanBark/ICanMeow are two manifestations of the same aspect (and should be instead handled by merging them into IMakeNoise).

Edit

I did consider having biteleash() in Animal which does nothing, and overriding it in Dog

It is a clunky solution and I believe it could only be motivated by performance reasons. If I'm not mistaken, Swing for Java uses such a pattern occasionally.

damix911
  • 4,165
  • 1
  • 29
  • 44