how to refactor the duplicate code?

Question

i know the duplicate is smell

but how to refactor the code?

public List<HighWay> updateAllNewHighWays(HighWayRepository repository)
            throws IOException {
        List<HighWay> highWays = new ArrayList<HighWay>();
        for (RoadCode code : RoadCode.values()) {
            try {
                pageParam.setRoadName(code);
                highWays.addAll(getAndSaveNewHighWay(repository));
            } catch (IOException e) {
                IOException exception = dealException(e, code);
                throw exception;
            }
        }
        return highWays;
    }

    public List<HighWay> getAllNewHighWays(HighWayRepository repository)
            throws IOException {
        List<HighWay> highWays = new ArrayList<HighWay>();
        for (RoadCode code : RoadCode.values()) {
            try {
                pageParam.setRoadName(code);
                highWays.addAll(getNewHighWay(repository));
            } catch (IOException e) {
                IOException exception = dealException(e, code);
                throw exception;
            }
        }
        return highWays;
    }

Answer 1

In order to remove duplication I used next algorithm:

1. Make sure you have tests which verify such functionality works or not (before refactoring all your tests should be running green)

2. Copy and paste duplicated logic into a new method (e.g. tobeReused, later you can rename it to a better one)

3. Parametrize what changes using lambdas (Java supports next types of lambdas: Runnable, Consumer, Predicate, Function)

4. Rename method in 1. To a better name. In order not to get stuck in analysis-paralysis, it is a good idea to rename things after you understand it.

---

1. A good test suite should

   1. Focus on behavioral tests

   2. Cover positive and negative scenarios

   3. Cover edge cases

   4. Cover exceptions

   5. Tests names should describe what is going to be tested not how

   6. Each test should be formatted following Given-When-Then or Arrange-Act-Assert.

   Example code structure for a possible test suite:

   public class HighwaysTest {
     //Repository is valid
     @Test
     public void whenHighWayRepositoryIsValidThenHighWaysShouldBeSaved() {
     }
   
     //Repository is invalid
     @Test
     public void whenHighWayRepositoryIsInvalidThenHighWaysShouldBeSaved() {
     }
   
     //Wrong road code
     @Test
     public void whenRoadCodeIsInvalidThenPageParamIsNotUpdated() {
     }
   
     //Function getAndSaveNewHighWay fails
     @Test
     public void whenGetAndSaveNewHighWayFailsThenExceptionIsThrown() {
   
     }
   
     //Function getNewHighWay fails
     @Test
     public void whenGetNewHighWayFailsThenExceptionIsThrown() {
   
     }
   }
   

2. Copy and Paste duplicated logic into a new method

    private List<HighWay> tobeReused(HighWayRepository repository) throws IOException {
           List<HighWay> highWays = new ArrayList<HighWay>();
           for (RoadCode code : RoadCode.values()) {
               try {
                   pageParam.setRoadName(code);
                   highWays.addAll(getAndSaveNewHighWay(repository));
               } catch (IOException e) {
                   IOException exception = dealException(e, code);
                   throw exception;
               }
           }
           return highWays;
       }
   

3. Parametrize what changes using lambdas (in this case Function<ParameterType, ReturnType>):

       private <P extends  HighWayRepository, 
                R extends  Collection> List<HighWay> tobeReused(P repository, Function<P, R> lambda) throws IOException {
   List<HighWay> highWays = new ArrayList<HighWay>();
               for (RoadCode code : RoadCode.values()) {
                   try {
                       pageParam.setRoadName(code);
                       highWays.addAll(lambda.apply(repository));
                   } catch (IOException e) {
                       IOException exception = dealException(e, code);
                       throw exception;
                   }
               }
               return highWays;
           }
   

4. As finding a good name can be tricky it is better to name things after you get a good grasp of the business domain. Then function tobeReused could be renamed to updatePageParamAndReturnHighways:

Your code will look like:

public List<HighWay> updateAllNewHighWays(HighWayRepository repository) throws IOException {
        Function<HighWayRepository, Collection> lambda = (repo) -> getAndSaveNewHighWay(repo);
        List<HighWay> highWays = updatePageParamAndReturnHighways(repository, lambda);
        return highWays;
    }

public List<HighWay> getAllNewHighWays(HighWayRepository repository) throws IOException {
        Function<HighWayRepository, Collection> lambda = (repo) -> getNewHighWay(repo);
        List<HighWay> highWays = updatePageParamAndReturnHighways(repository, lambda);
        return highWays;
    }

Single Responsibility Principle (SRP)

You might want to consider to refactor your functions updateAllNewHighWays and getAllNewHighWays as they have more than one responsibility (update pageParam and return highways).

Answer 2

Since the only part that changes is the inside of a loop, you can refactor out the looping part any only have the part inside the loop change.

If you use Java 8 you can pass in the getAndSaveNewHighWay(repository) or getNewHighWay(repository) as method a method reference as a Function<HighWayRepository, List<HighWay>> implementation

public List<HighWay> handleHighways(HighWayRepository repository, Function<HighWayRepository, List<HighWay>> function){
  List<HighWay> highWays = new ArrayList<HighWay>();
        for (RoadCode code : RoadCode.values()) {
            try {

                pageParam.setRoadName(code);
                //call our method 
                highWays.addAll(function.apply(repository));
            } catch (IOException e) {
                IOException exception = dealException(e, code);
                throw exception;
            }
        }
        return highWays;
}

Then in your calling code:

 List<HighWay> highways = handleHighways(repository, MyClass::getAndSaveNewHighWay);

or

List<HighWay> highways = handleHighways(repository, MyClass::getNewHighWay);

Without Java 8, you can achieve something similar by making your own interface that has a method that takes a HighWayRepository and return a List<HighWay> and then write 2 different implementations