Interface Segregation Principle in constructor

Posted on

Problem

I’m wondering if providing default behavior is ever worth the confusion caused by ignoring generic type passed in. Another question brought up this issue:

public class Activate<D extends CanQuack & CanWalk> {
    D d;

    //Must match D's type        
    Activate(D d) {
        this.d = d;
    }

    //Completely ignore D's type 
    Activate() {
        this.d = (D)new MyWaterFowl(); //Provide default behavior.   
    }

    void setDuckLikeThing(D d) {
        this.d = d;
    }

    void doDuckLikeThings() {
        d.quack();
        d.walk();
        //d.swim(); //Doesn't work 
        //And shouldn't since that was the whole point of ISP.
    }
}

interface CanWalk { public void walk(); }
interface CanQuack { public void quack(); }
interface CanSwim { public void swim(); }

public class MyWaterFowl implements CanWalk, CanQuack {

    public void walk() { System.out.println("I'm walkin` here!"); }
    public void quack() { System.out.println("Quack!"); }
    public void swim() { System.out.println("Stroke! Stroke! Stroke!"); }
}


public class MainClass {

    public static void main(String[] args) {
       //Works
       Activate<MyWaterFowl> a = new Activate<>(new MyWaterFowl());
       a.doDuckLikeThings();

       //Works
       Activate<MyAmericanEagle> ae = new Activate<>( new MyAmericanEagle() );
       ae.doDuckLikeThings();

       //Works but nothing to do with MyAmericanEagle is happening here 
       Activate<MyAmericanEagle> ad = new Activate<>();
       ad.doDuckLikeThings();

       //Rightly produces an error
       //Activate<MyAmericanEagle> aw = new Activate<>( new MyWaterFowl() );
       //ae.doDuckLikeThings();

       //Rightly produces an error
       //Activate<MyWaterFowl> wa = new Activate<>( new MyAmericanEagle() );
       //ae.doDuckLikeThings();

    }
}

So would you ever want a library to leave you wondering why MyAmericanEagle gets completely ignored just to provide some default behavior?

Solution

Like @rolfl, I see no reason why you should offer a default constructor at all. The default constructor creates nothing but problems.

Activate is a poor class name. Class names should be nouns, not verbs. What is the purpose of this class? I suggest naming it DuckAdapter, since it makes any quacking and walking creature act like a duck.

d should be private.

The constructor and setDuckLikeThing() have the same code, so the constructor should call setDuckLikeThing() to do its work.

No, this is all broken. The basic issue is the cast-to-D in the following line:

this.d = (D)new MyWaterFowl();

That gives the warning:

Type safety: Unchecked cast from MyWaterFowl to D

Ignoring Generics warnings normally means you are doing something wrong. In this case, you are.

You are doing a cast to a type where you have no certainty, in fact, you are certain that it will fail….

…. the solution here, is to not have a default constructor. It does not make sense for you. Why do you think you need one?

Also, why does MyAmericanEagle extend CanQuack ?

Style comment: if you are going to provide a default constructor, it should invoke the more explicit constructor.

public Activate() {
    this(new MyWaterFowl());
}

Generics: largely, you reach for generics so that the calling code can verify types at compile time. If you are implementing an interface where the clients don’t need to know anything about types after the object is constructed, then generics aren’t buying you anything.

Putting this another way: if the problem calls out for the StrategyPattern, Generics are not a good solution.

In your example – none of your methods really need to know the type; they never get the object back out of the container, or need to worry about passing a container to you, or anything like that.

In other words, there’s no point in this code

public class Activate<D extends CanQuack> {
    D quacker;
    Activate(D quacker) {
        setQuacker(quacker);
    }

    public void setQuacker(D quacker) {
        this.quacker = quacker;
    }

    public void quack() {
        quacker.quack();
    }
}

vs.

public class Activate {
    CanQuack quacker;
    Activate(CanQuack quacker) {
        setQuacker(quacker);
    }

    public void setQuacker(CanQuack quacker) {
        this.quacker = quacker;
    }

    public void quack() {
        quacker.quack();
    }
}

The only reason for the generics here is to use both interfaces on the same object

If you refactor your example, you can get something cleaner (which may or may not support your real life case). First, isolate the implementation, which really doesn’t seem to care about the fact that the interfaces are supported by the same object.

public class ActivateImpl {
    // normally, best practices would call for these to be final.  Are
    // you really sure you need set() ?
    CanQuack quacker;
    CanSwim  swimmer;

    public ActivateImpl(CanQuack quacker, CanSwim swimmer) {
       // ...
    }

    // Useful bits....
}

Then save the generics for the funky construction idiom that you are using

public class Activate<D extends CanQuack & CanSwim> extends Activate {
    public Activate(D duck) {
      super(duck,duck);
    }

    // if you really need to rebuild the object graph (unlikely)
    public void setDuck(D duck) {
        super.setQuacker(duck);
        super.setSwimmer(duck);
    }
}

Leave a Reply

Your email address will not be published. Required fields are marked *