Recently, I was discussing with another programmer the best way to refactor a huge(1000 lines) method full of "if" statements.
The code is written in Java, but I guess this issue could happen in other languages such as C# as well.
To solve this problem, he suggested using a chain-of-responsibility pattern.
He proposed having a base "Handler" class. Then, "Handler1", "Handler2", etc. would extend "Handler".
Then, handlers would have a "getSuccessor" method, which would either return null(if it was the last of the chain) or the next Handler of the chain.
Then, a "handleRequest(Request)" function would either deal with Request, or pass it to the next of the chain and, if none of the previous solutions worked, it would return just null or throw an exception.
To add a new Handler to the chain, the coder would go to the last element of the chain and tell it there was a new element. To do something, he'd just call handleRequest on the first element of the chain.
To solve this problem, I suggested using a different approach.
I'd have a base "Handler" class as well, with "Handler1", "Handler2", just like the previous method mentioned.
However, there would be no "getSuccessor" method. Instead, I'd have a Collection class with a list of handlers(a Vector, an ArrayList, or whatever is best in this case).
The handleRequest function would still exist, but it wouldn't propagate the call to the next handlers. It would just process the request or return null.
To handle a request, one would use
for(Handler handle : handlers){
result = handle.handleRequest(request);
if(result!=null) return result;
}
throw new CouldNotParseRequestException(); //just like in the other approach
Or, to prevent code duplication, a "parseRequest(request)" method could be added to the collection class. To add a new handler, one would go to the collection constructor(or static{} block, or something equivaleng) and simply add the code "addHandler(new Handler3());".
Exactly what advantages of chain-of-responsibility am I missing with this approach? Which method is best(assuming there is a best method)? Why? What potential bugs and issues can each design method cause?
For those who need context, here is what the original code looked like:
if(x instanceof Type1)
{
//doSomething1
} else if(x instanceof Type2)
{
//doSomething2
}
//etc.