Avoid casting and instanceOf

Question

I got an interface

public interface Details {
   // nothing needed until now
}

which is used in a class like the following:

public class Value {
    // many fields
    private Details details;

    public Value(SomeType type) {
        switch (type) {
        case TYPE_1:
        case TYPE_2:
            this.details = new DetailsA();
            break;
        case TYPE_3:
            this.details = new DetailsB();
            break;
        default:
            throw new NotImplementedException("not yet implemented");
        }
    }

    public Details getDetails() {
        return this.details;
    }
}

The interface has two implementations

public class DetailsA implements Details {

    private BigDecimal betragA;

    public DetailsA() {
    }

    public BigDecimal getBetragA() {
      return this.betragA;
    }

    public void setBetragA(BigDecimal betragA) {
      this.betragA = betragA;
   }

}

public class DeailsB implements Details {

    private BigDecimal betragB;
    private boolean booleanB;

    public BetragB() {
    }

    public BigDecimal getBetragB() {
        return this.betragB;
    }

    public void setBetragB(BigDecimal betragB) {
        this.betragB = betragB;
    }

    public boolean isBooleanB() {
        return this.booleanB;
    }

    public void setBooleanB(boolean booleanB) {
        this.booleanB = booleanB;
    }

    // some more fields

}

I got a model class in which I want to use those details, depending on the instance.

 public class Model extends AbstractModel {

    private Details details;

    public void init(StoerungValue stoerung) {
        setDetails(stoerung.getSchaden().getDetails());
    }

    private void setDetails(Details details) {
        this.details = details;
    }
    // ...

In there I have some operations like the following

    // ...  
    public void setBooleanB(boolean booleanB) {
        if (details instanceof DetailB) {
            ((DetailB) details).setBooleanB(booleanB);
        }
    }
    // ...

How can I avoid this casting and instanceOf stuff? Is any of the design patterns applicable here?

Answer 1

I think the problem you have here is a collection of design smells. You've painted yourself into a corner, and there may not be an easy way out. I don't know if this solution will work for you or not, but you can at least consider this.

The first design smell is that you have created an inheritance relationship where none actually exists. In short, the hierarchy rooted at Details violates the Liskov Substitution Principle. When a class claims (as Model does) to support the Details interface, it's making the claim that any implementation of Details will do. The program's correctness and behavior shouldn't change whether it's given a DetailsA, a DetailsB, or some FooDetails class that hasn't even been invented yet.

The reality is that DetailsA and DetailsB are not actually related. You can see this because Details has no methods, and thus may as well be Object which any two classes already inherit from.

The second design smell is "Feature Envy." It seems that many methods of Model are just pass-through calls to its underlying details property. You could consider, rather than having setBooleanB on Model to just provide a getDetails method, and then let the caller work directly on the Details object. This won't remove the instanceof checks or casting, but it will move them out of this class.

The third thing here is related two the first two. Model depends not on Details as its property types would tell you, but rather on (at least) DetailsB. If that's the case, then its property type should say so. Now, it's possible that sometimes you need a Model with a DetailsA and sometimes you need a Model with a DetailsB, but it can't be both at the same time. In that case, you can work around the issue with generics.

First, make the Model class generic, with a type parameter that tells what its underlying Details must actually be.

public abstract class Model<T extends Details> {
    private T details;

    public void init(T dets) {
        setDetails(dets);
    }

    public void setDetails(T dets) {
        this.details = dets;
    }

    public T getDetails() {
        return this.details;
    }
}

Then, create two subclasses that are bound to different Details types, and can thus promise to do the right thing without requiring casting or instanceof calls.

public class ModelA extends Model<DetailsA> {
    public BigDecimal getBetragA() {
        return this.getDetails().getBetragA();
    }
}

public class ModelB extends Model<DetailsB> {
    public boolean getBooleanB() {
        return this.getDetails().isBooleanB();
    }

    public void setBooleanB(boolean boolB) {
        this.getDetails().setBooleanB(boolB);
    }
}

I'm not sure if that'll fix your problem or not, but it's something to consider.