Refactor method in Java: What pattern should I apply?

Question

I developed an application in Java and I am refactoring it. I've just realized that I have several methods that perform almost the same action, and they have similar names as well:

RestrictedPersonServiceImpl.canViewPersonDetails

RestrictedPersonServiceImpl.isSubjectRestrictedToWorker

RestrictedPersonServiceImpl.isAnySubjectRestricted 

RestrictedPersonServiceImpl.isSubjectRestricted 

RestrictedPersonServiceImpl.isAnySubjectOfSubgroupRestrictedToWorker

I am pretty sure that it has to be a programming pattern to apply to deal with this situation. The solution that came to my mind before is to merge all of these methods in one and determining the behaviour by a parameter.

Is there a better choice?

Thanks everyone.

Answer 1

In most cases merging several methods into one is a bad choice. Here are some reasons.

1. Code should be simple. The less if/else or switch/case structures you have the better. This code measurement is called "branching factor"
2. It is easier to remove full method (if needed) than to remove a part of specific method.
3. It is much cleaner and easier to explain in javadoc. Moreover in most cases it is not needed to explain anything because the method name explains itself.

Other case is if all your methods do the same operation that can be controlled by parameter but without if/else structures. For example it is useless to have series of methods like addOne(), addTwo() etc. In this case you should define method add(int value) that does the job.

Sometimes it is useful to define private method that accepts parameter but expose a series of public methods without parameters that call this private method with correct parameter. This is typically useful when there is some difference in error handling between these methods or if there are more than one parameter and not all combinations are legal.

Answer 2

IMO, almost the same is different from exactly the same. If they are truly almost the same with very localized differences in the behavior of their algorithm, a combination of Template Method Pattern and Strategy Pattern comes to mind, where you use the former to provide a template skeleton, and the latter to alter the behaviour at runtime.

Now, I assume you have an interface, named RestrictedPersonService, which your RestrictedPersonServiceImpl implements. And clean, proper interface design-by-contract is more important IMO because that's what will be exposed to your user. Take a look at Uncle Bob's Interface Segregation Pinciple. The gist of it is to not let your interface knows too much. In other words, don't couple your interface implementations with other implementations of the system that are irrelevant to it. Each interface, ideally, should have only one responsibility i.e. Single Responsibility Principle. So again, I personally won't merge all those methods together unless they share exactly the same responsibility.