Code refactoring help

Question

I have an example like this

class A {

   A() {}
   public C createC () {
      ...
   }

} 

class B {

   B() {}
   public C createC () {

   }
}

Objects for A and B are created based an an

public enum D { ii, jj };

And I see code all over the place like

D d; 

switch (d) {
    case ii: (new A()).createC(); break;
    case jj: (new B()).createC(); break;
 };

How can I avoid the switch cases all over the place? I understand the code is not so clear.

Answer 1

You could just create a Factory class:

public class ClassFactory {

     public static C createC(D type) {
         C c = null;
         switch (type) {
              case ii: 
                  c = new A(); 
                  break;
              case jj: 
                  c = new B(); 
                  break;
         };

         return c;

     }
}

Then in your code just do:

C c = ClassFactory.createC(type);

Very similar to what the Wiki entry for Factory method pattern is doing with their PizzaFactory.

Answer 2

I don't see a refactoring opportunity here, as such.

I see the need for a factory. Look up the "factory" design pattern.

And I see code all over the place like... (followed by a conditional wherein a specific instance is created)_ is a huge tip-off.

Basically, what you want to do it push all the ugliness of deciding which class to instantiate into one place. First, in your example, A and B should share a superclass or better yet implement an interface in which public C createC() is defined.

The factory is nothing more than a class with a static method that makes the decision on which A or B to instantiate.

    public class SomeFactory {
        public static YourInterface make(int someNumber) {
            switch (someNumber) {
                case 1: return new A();
                case 2: return new B();
                default:
                    throw new RuntimeException("unknown type");
                }
            }
        }

So what you do is:

YourInterface yi = SomeFactory.make(1);
C c = yi.createC();

Or

C c = SomeFactory.make(1).createC();

And now your code is pretty clean. Create a new implementation of SomeInterface? No problem, add its creation to the factory and your code still works. If for your application make() is costly, no problem, just make one and use it repeatedly.

Answer 3

Add the createC method to the enum D.

public enum D {
  ii,
  jj;

  public C createC() {
    switch (this) {
      case ii:
        return new A().createC();
      case jj:
        return new B().createC();
    }
    return null;
  }
}

Then just call D.createC().

Answer 4

I would be inclined to add the create code to the enum.

enum D {
    ii {
        public void createC() { new A().createC(); }
    },
    jj {
        public void createC() { new B().createC(); }
    };

    public abstract void createC();
}

You can then replace the switch with

d.createC();

Having said that I would also look at making the createC method static or moving the createC code to the enum rather than leaving it in the A and B classes.

If you make the createC method in A and B static the enum would look like

enum D {
    ii {
        public void createC() { A.createC(); }
    },
    jj {
        public void createC() { B.createC(); }
    };

    public abstract void createC();
}

Answer 5

You give a switch statement as an example, but as the other conditionals related to the switch are not shown I think its premature to suggest a specific solution. Here's a link though if you want to consider a range of responses to "switch" statements: http://books.google.com/books?id=1MsETFPD3I0C&lpg=PP1&dq=refactoring&pg=PA82#v=onepage&q&f=false

If the other conditionals are really just selecting on the same criteria to construct one of the objects, use extract method.