Reputation: 173
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
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
Reputation: 18123
Introduce caching/singletons in your factory, that you only return an algorithm once. Also, make your factory a singleton.
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
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