CODEWITHSUNDEEP
javaoopdesign-patternsdynamic
Girish
Girish

Reputation: 1717

Make factory method more dynamic

I am implementing the factory pattern in my code, so came across one interesting thing in factory that I can replace the if else condition in the factory method with Reflection to make my code more dynamic.

Below is the code for both the designs......

1) With if-else conditions

   public static Pizza createPizza(String type) {

        Pizza pizza = null;

            if(type.equals(PizzaType.Cheese))
            {
                pizza = new CheesePizza();
            }

            else if (type.equals(PizzaType.Tomato))
            {
                pizza = new TomatoPizza();
            }

            else if (type.equals(PizzaType.Capsicum))
            {
                pizza = new CapsicumPizza();
            }
            else
            {
                try {
                    throw new Exception("Entered PizzaType is not Valid");
                } catch (Exception e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }
            }


        return pizza;
    }

2) With Reflection 

    public static Pizza createPizza(String type) {

            Pizza pizza = null;

            for(PizzaType value : PizzaType.values())
            {
                if(type.equals(value.getPizzaTypeValue()))
                {
                    String fullyQualifiedclassname = value.getClassNameByPizzaType(type);

                    try {
                        pizza = (Pizza)Class.forName(fullyQualifiedclassname).newInstance();
                    } catch (InstantiationException | IllegalAccessException
                            | ClassNotFoundException e) {
                        // TODO Auto-generated catch block
                        e.printStackTrace();
                    }
                }
            }

            return pizza;
        }

Second way looks very good to me as I can make my code more dynamic by using it, as I can create one property file with the names of classes and type associated with it to serve the Open and close a bit more, as if in future owners wants to add more pizzas to PizzaStore then he would just make the entry in the property file and will just create one more subclass of Pizza.

But I read that reflection has many disadvantages mentioned few of them.

It is hack of compiler
Automatic development tools are not able to work with reflections code
It's difficult to debug reflections code
Reflection complicates understanding and navigation in code
Significant performance penalty

So very curious to know that, which design is good, as I am very interested to make my code more and more dynamic.

Upvotes: 3

Views: 3166

Answers (5)

Marcin Szymczak
Marcin Szymczak

Reputation: 11433

If you have only few classes returned by factory (let's say three or four) you should just leave it in simplest form with if statements

if(type.equals("someType"){
    return new SomeType();
} else if(type.equals("otherType"){
    return new OtherType();
}

If you have more class types or you predict frequent changes in this hierarchy then you should switch to implementation where factory class will have collection of available implementations. Your factory class will have field with possible instance types.

public class MyFactory{
    private Map<String, Class<? extends ReturnedClass> availableTypes;
}

It might be problematic how to fill this map with elements. You may:

  • Hardcode them inside constructor/ init method inside Factory. Note: each new added class will require changes in factory.
  • Make method registerNewReturnedType(Class c) inside factory. Each returned class will have to register itself inside factory. Note: after adding new class you will not have to change factory
  • (Recommended) Use dependency injection to fill map.

Upvotes: 0

Nick Mulder
Nick Mulder

Reputation: 1

The if - else statements won't contribute to your design. This merely ensures that the pizza's exist - in a inheritance hierarchy. Try to use a decorator pattern to minimise the hierarchy levels and use a concrete class (like: plainPizza) to add your decorations to.

The decorator pattern is aimed at adding decorations dynamically in run-time.

Upvotes: 0

Ren&#233; Link
Ren&#233; Link

Reputation: 51353

You can also use a design in the middle. E.g.

public interface Factory<T> {
  public T newInstance();
}

public class TomatoPizzaFactory implements Factory<TomatoPizza> {

    @Override
    public TomatoPizza newInstance() {
        return new TomatoPizza();
    }
}

public class PizzaFactory {

    private Map<String, Factory<? extends Pizza>> factories = new HashMap<String, Factory<? extends Pizza>>();

    public PizzaFactory(){
        factories.put(PizzaType.Cheese, new CheesePizzaFactory());
        factories.put(PizzaType.Tomato, new TomatoPizzaFactory());
    }

    public Pizza createPizza(String type){
        Factory<? extends Pizza> factory = factories.get(type);
        if(factory == null){
           throw new IllegalArgumentException("Unknown pizza type");
        }
        return  factory.newInstance();
    }
}

Implement a DefaultConstructorFactory for simple instantiations.

public class DefaultConstructorFactory<T> implements Factory<T> {

    private Class<T> type;

    public DefaultConstructorFactory(Class<T> type) {
      this.type = type;
    }

    public T newInstance() {
       try {
         return type.newInstance();
       } catch (InstantiationException e) {
         throw new IllegalStateException("Can not instantiate " + type, e);
       } catch (IllegalAccessException e) {
         throw new IllegalStateException("Can not instantiate " + type, e);
    }
  }
}

But I read that reflection has many disadvantages mentioned few of them.

It is hack of compiler

It can be a hack, but if you are writing infrastructure code you will often use reflections. Especially when you write frameworks that must introspect classes at runtime, because the framework doesn't know the classes it will handle at runtime. Think about hibernate, spring, jpa, etc.

Automatic development tools are not able to work with reflections code

That's true, because you shift a lot of compiler issues to runtime. So you should handle reflection exceptions like a compiler and provide good error messages.

There is also only a little refactoring support in some IDEs. So you must be careful when changing code used by reflection code.

Nevertheless you can write tests to find bugs fast.

It's difficult to debug reflections code

That's also true, because you can't jump into a method directly and you have to investigate the variables to find out which member of which object is accessed. It is a bit more indirection.

Reflection complicates understanding and navigation in code

Yes, if it is used the wrong way. Only use reflection if you do not know the types you have to handle at runtime (infrastructure or framework code). Don't use reflection if you know the types and you only want to minimize code written. If you want to minimize code written select a better design.

Significant performance penalty

I don't think so. Of course reflection calls are indirect method invokations and thus more code must be executed in order to do the same as a direct method call. But the overhead for this is minimal.

Upvotes: 2

Zhedar
Zhedar

Reputation: 3510

Reflection seems like an overkill to me in this situation. Why don't you add a factory method Pizza create(); to your PizzaType interface?

Then just make every implementing class do the initialization, like:

class CheesePizza implements PizzaType {
    Pizza create() {
        return new CheesePizza();
    }
}

So you only end up with:

for(PizzaType value : PizzaType.values()
    if(type.equals(value))
        pizza = value.create();

This way the loop in the first example wouldn't be ineffective and it is expandable easily .

Also you shouldn't worry about reflection performance too much, if your code hasn't a need to perform in realtime. I agree, that it makes the code unreadable though, so avoiding it is better in most cases.

Edit: I've overseen, that PizzaType is an enum, not an Interface. In this case you may create one or add a create method to your enum, but I don't think the latter is very favourable.

Upvotes: 1

janos
janos

Reputation: 124646

Using reflection this way is not recommended in general, but if creating objects dynamically is a top priority in your design, then it can be acceptable.

Another good option might be to not use different Pizza subtypes, but a single Pizza class that can be parameterized. That would make a lot of sense, since I don't think pizzas behave so very differently. It seems to me that a diverse range of pizzas could be easily created using the builder pattern.

Btw, another bad practices catches the eye here, that the parameter of the factory method is a String, when you actually seem to have PizzaType which is an enum, and you compare the String parameter to enum values. It would be much better to use enum values as the parameter, and a switch instead of the chained if-else. But, another problem here is that the enum duplicates the available pizza types. So when you add a new pizza type, you also have to update the enum, and the factory method. That's too many files changed. This is code smell.

I suggest to give a try to the single Pizza class and the builder pattern.

Upvotes: 1

Related Questions