John Smith
John Smith

Reputation: 173

Refactoring predecessor code

I'd like to ask for help and some suggestion how to refactor source code which I receive. Here is pseudocode of my method:

    public void generalMethod(String type) {

     InputParameters params = new InputParameters();

      if (type.equals("someKey1"){
             decodeSomeKey1(params);
          } else if (type.equals("someKey2"){
             decodeSomeKey2(params);
          } else if (type.equals("someKey3"){
             decodeSomeKey3(params);
          } else if (type.equals("someKey4"){
             etc...
          }
      }
    }

All methods have the same input parameters. In first step I created new interface and created for each method separate class which implements created interface.

interface ISomeInterfaceDecoder {
    void decode(InputParameters params);
}

class DecodeSomeKey1 implements ISomeInterfaceDecoder {

    @Override
    public void decode(InputParameters params) {
        // some implementation
    }
}

class DecodeSomeKey2 implements ISomeInterfaceDecoder {

    @Override
    public void decode(InputParameters params) {
        // some implementation
    }
}   

Then I created factory class as follows:

class Factory {

    ISomeInterfaceDecoder getDecoder(String type) {
         if (type.equals("someKey1"){
             return new DecodeSomeKey1();
          } else if (type.equals("someKey2"){
             return new DecodeSomeKey2();
          } else if (type.equals("someKey3"){
             return new DecodeSomeKey3());
          } else if (type.equals("someKey3"){
             etc...
          }
      }
    }

}

After these changes the code looks like this:

class SomeClass {

    Factory factory = new Factory();

    public void generalMethod(String type) {
         InputParameters params = new InputParameters();
         ISomeInterfaceDecoder decoder = factory.getDecoder(type); 
         decoder.decode(params);
       }
}

Code of this method looks better but... This method is called very very often. Each time a new instance of the given class is created. This can cause performance problems. So, I think it's not good approach to this problem. Can you give me some suggestion how I should to refactor this code? Thanks in advance for help.

Upvotes: 1

Views: 107

Answers (3)

Paul S
Paul S

Reputation: 494

Instead of having a key as a String, make it an enum. Then in the enum you can implement the decode() method like this:

public enum MyKeyEnum {
    VALUE1 {
        public void decode(InputParameters ip) {
            // do specific decoding for VALUE1
        }
    },
    VALUE2 {
        public void decode(InputParameters ip) {
            // do specific decoding for VALUE2
        }
    }
    ...
    ;

    public abstract void decode(InputParameters ip);
}

Now in the calling code you can do something like this:

public void generalMethod(MyKeyEnum type) {
   InputParameters params = new InputParameters();
   type.decode(params);
}

The advantage is that all the decode methods are in 1 enum, you dont need a specific class for each of the decoders. Also when a new value is added to the enum, you cannot forget to implement the decode method (or it will not compile).

Upvotes: 2

Smutje
Smutje

Reputation: 18123

  1. Introduce caching/singletons in your factory, that you only return an algorithm once. Also, make your factory a singleton.

  2. Create a static Map<String, ISomeInterfaceDecoder> where you map the identifier to algorithms executing the call which means no factory class and no algorithm instantiation. Works only, if you have stateless algorithms.

Upvotes: 0

Brian Agnew
Brian Agnew

Reputation: 272217

Can you give me some suggestion how I should to refactor this code?

I see no mention of automated regression testing, and that would be my first step, to put in a test suite (via, say, JUnit or TestNG) before going further.

After that, I'd perhaps introduce a Map of String keys to Decoder objects.

But put the test framework in first. Otherwise you'll never really know if you've introduced bugs or different modes of operation.

Upvotes: 1

Related Questions