CODEWITHSUNDEEP
java
IAE
IAE

Reputation: 2243

How to Consolidate Many Classes and Avoid instanceof?

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, UConstraintand 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?

Upvotes: 4

Views: 223

Answers (5)

Eugene Kuleshov
Eugene Kuleshov

Reputation: 31795

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.      
  }
}

Upvotes: 3

Kumar Vivek Mitra
Kumar Vivek Mitra

Reputation: 33534

Use this principle

Program in interfaces, and encapsulate the behaviors that keeps changing in abstract class or Interfaces.

eg: For your above given example

Interface - Constraint

Abstract Class - FConstraint

Concrete Class - PConstraint, TConstraint, UConstraint, AConstraint

Upvotes: 3

raddykrish
raddykrish

Reputation: 1866

Have the Constraint as an Interface. Define an abstract class which extends this Constraint and this should have the shared variables. UConstraint, AConstraint should extend this abstract class which has the shared variables. The rest of the classes can directly implement the Constraint interface. The instance of code should be changed like 

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

Always the better design is to write code against an interface

Upvotes: 1

Hunter McMillen
Hunter McMillen

Reputation: 61510

You could do something like this:

public interface Constraint{}

public abstract class VariableConstraint implements Constraint 
{ 
  /* hold 3 shared variables here */
}

public class UConstraint extends VariableConstraint{}
public class PConstraint extends VariableConstraint{}
public class AConstraint extends VariableConstraint{}

public abstract class EmptyConstraint implements Constraint {}

public class TConstraint extends EmptyConstraint {}

Upvotes: 2

Attila
Attila

Reputation: 28762

Specify common functionality in Constraint: any operation that you want to be able to perform on a Constraint object (which could in fact be an object of one of the sub types), you have a method for that functionality in Constraint that you override in subclasses.

You can have trivial (e.g. empty) implementation for the parent classes where that functionality does not make sense.

This way you do not need to concern yourself with the concrete class of the object, just use the facility provided by the superclass

Upvotes: 1

Related Questions