How to Consolidate Many Classes and Avoid instanceof?

java

Solution

Your question don't have enough information about actual work you need to do in each case, but generally such code:

Object constraint = item.getConstraint();

if (constraint instanceof AConstraint) {
    // Work
} else if (constraint instanceof PConstraint) {
    // Work
} // etc.

is a strong smell to use polymorphism and refactor into a something like this:

Constraint constraint = item.getConstraint();
constraint.doWork(...);

where specific classes would look something like this:

public class AConstraint {
  public ... doWork(...) {
    if (getValue() == null) {
      setValue(string);
    } else if (getKey() == null) {
      setKey(string);
    } // More of the same.      
  }
}

Problem

The program I'm helping to develop is supposed to output several dynamically generated questions for the user to answer. The questions are of different types and have a corresponding class `Constraint` that they fill with the user-given information. My question is how to create uniform behavior for the different constraints. ``` ---->Constraint<-------------------- | | | FConstraint PConstraint TConstraint | | UConstraint AConstraint ``` The base class `Constraint` is empty, as is TConstraint. `UConstraint`, `PConstraint` and `AConstraint` share three variables. However, `UConstraint`and `AConstraint` have one additional variable that `PConstraint` doesn't have. I feel like I'm trying to hammer through a brick wall with some pincers. My one thought is to provide an abstract method to Constraint with the signature: ``` // All answers are of type string. abstract void setValue(string variable, string answer); ``` Which are implemented by every `Constraint` subclass. However, passing a string to determine which variable to set seems error-prone and a similarly bad code smell. The second option was to move the three similar variables up into Constraint, but that still leaves `UConstraint, AConstraint` with that extra bit of information I might need to set. It doesn't help that `TConstraint` doesn't need any of those. My current brute-force "Screw this design." solution is an `instanceof` soup in which I check and fill in the missing Constraint-specific information. ``` Object constraint = item.getConstraint(); if (constraint instanceof AConstraint) { AConstraint constraint = (AConstraint) constraint; if (constraint.getValue() == null) { constraint.setValue(string); } else if (constraint.getKey() == null) { constraint.setKey(string); } // More of the same. } else if (constraint instanceof PConstraint) { // As the above if() group. } // etc. ``` Is there a better solution to this design than the abstract function?

Original source