Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Avoid If-else code smell with creation of objects which depend upon specific conditions

Is there a better way to deal with an instanciation of an object (Product) which depends upon another object type (Condition) than using if-else paired with instanceof as the following code shows?

import java.util.ArrayList;
import java.util.List;

abstract class AbstractProduct {
    private AbstractCondition condition;
    public AbstractProduct(AbstractCondition condition) {
        this.condition = condition;
    }
    public abstract void doSomething();
}

class ProductA extends AbstractProduct {
    AbstractCondition condition;
    public ProductA(AbstractCondition condition) {
        super(condition);
    }

    @Override
    public void doSomething() {
        System.out.println("I'm Product A");
    }
}

class ProductB extends AbstractProduct {    
    public ProductB(AbstractCondition condition) {
        super(condition);
    }   

    @Override
    public void doSomething() {
        System.out.println("I'm Product B");
    }
}

class AbstractCondition { }

class ConditionA extends AbstractCondition { }

class ConditionB extends AbstractCondition { }

public class Try {
    public static void main(String[] args) {
        List<AbstractCondition> conditions = new ArrayList<AbstractCondition>();
        List<AbstractProduct> products = new ArrayList<AbstractProduct>();

        conditions.add(new ConditionA());               
        conditions.add(new ConditionB());               
        conditions.add(new ConditionB());               
        conditions.add(new ConditionA());

        for (AbstractCondition c : conditions) {
            tryDoSomething(c);
        }
    }

    public static void tryDoSomething(AbstractCondition condition) {
        AbstractProduct product = null;
        if (condition instanceof ConditionA) {
            product = new ProductA(condition);
        } else if (condition instanceof ConditionB) {
            product = new ProductB(condition);
        }
        product.doSomething();
    }
}

The difference with the code above of my real code is: I have NO direct control over AbstractCondition and its subtypes (as they are in a library), but the creation of a concrete subtype of AbstractProduct depends on the concrete condition.

My goal being: try to avoid the if-else code smell in tryDoSomething().

I would also like to avoid reflection because it feels like cheating and I do think it's not an elegant, clean and readable solution.

In other words, I would like to tackle the problem just with good OOP principles (e.g. exploiting polymorphism) and pheraps some design patterns (which apparently I don't know in this specific case).

like image 448
1d0m3n30 Avatar asked Oct 27 '16 09:10

1d0m3n30


People also ask

Is if else a code smell?

If-Else is often a code smell that can signal bad design decisions and the need for refactoring.

What is a code smell stackoverflow?

Background: Code smells indicate potential design or implementation problems that may have a negative impact on programs. Similar to other software artefacts, developers use Stack Overflow (SO) to ask questions about code smells.


1 Answers

Since you can't edit the original objects, you need to create a static map from condition type to product type:

private static HashMap< Class<? extends AbstractCondition>, 
                        Class<? extends AbstractProduct>
                      > conditionToProduct;`

Fill it in static initialization with the pairs of Condition,Product:

static { 
  conditionToProduct.put(ConditionA.class, ProductA.class); 
  ... 
} 

and in runtime just query the map:

Class<? extends AbstractProduct> productClass = conditionToProduct.get(condition.getClass());
productClass.newInstance();
like image 166
Shloim Avatar answered Sep 23 '22 13:09

Shloim