Appropriate Java design pattern to avoid method duplication

Question

I have this scenario. I started working with a system that 'process' documents. The problem is, it seems to be the typical scenario where it started small, and went getting bigger and bigger constructing it one chunk at a time and now it needs to be refactored.

Each document type has an identifier (docID), and all of them share the same underlying result structure.

There is a huge master class that does all the job BUT inside this class there are several methods (almost one for each site) with its own logic. They all do almost the same with slight changes (i.e. formatting a string before setting it to a field in the result structure or doing some calculation and then setting the field in the result structure).

For example:

private Result processDocGeneric(Result result){
            result.setField1("value1");
            result.setField2("value2");
            result.setField3("value3");
            return result;
        }

private Result processDoc1(Result result){
            result.setField1("VALUE1");
            return result;
        }

private Result processDoc2(Result result){
            result.setField2("V-A-L-U-E-2");
            return result;
        }

private void processDocs(){
            Result result = new Result();
            result = processDocGeneric(result);
            if(docID == 1){
                result = processDoc1(result);
            }
            else if(docID == 2){
                result = processDoc2(result);
            }
            ...
        }

Ok, so I'm planning to refactor this and I'm considering some design patterns I know but I don't want the feel that I'm killing a roach with a bazooka.

Command pattern is maybe the first that comes to my mind, also Strategy pattern. My major concern with those is that I will have to create a class for every document type that has its own implementation of the processDoc method (There are around 15 at the moment). I mean, if that's the way to go, that would be it but if there's a simpler way of doing it that I don't know, it would be better (since the change is in a single method).

The other thing that I could do is moving all those method to a 'methods' class, and also move the if-else block to a single method with a docID parameter (process(int docID) and then call it from the main class. But that's just splitting the huge class. It would be "cleaner" but not optimal.

What would be the best approach to clean and split this huge class and make it scalable (since there would be new document types to be added in the future)?.

Answer 1

You can use factory or abstract factory design patterns maybe, In this patterns you can get your needed objects without having to specify the exact class of the object that will be created.

Answer 2

I propose a solution based on the Visitable / Visitor Pattern. this solution requires very little change to the Result class, while opening the door to new visiting objects, making it an easily extensible framework. I'm making heavy use of Java8's default interface method.

The Visitor / Visitable Interfaces:

public interface DocVisitor<T extends VisitableDoc> {
    default void visit(T document){
        switch(document.getDocId()){
            case 1:
                processDoc1(document);
                break;
            case 2:
                processDoc2(document);
                break;
            // ... other cases...
            default:
                processDocGeneric(document);
                break;
        }
    }
    void processDocGeneric(VisitableDoc document);
    void processDoc1(VisitableDoc document);
    void processDoc2(VisitableDoc document);
}

public interface VisitableDoc {
    int getDocId();
    default void visit(DocVisitor visitor){
        visitor.visit(this);
    }
}

Slight modification of the Result class:

public class Result implements VisitableDoc { // New interface declared
    int getDocId(){
        return docId; // This might already exist
    }
    // Rest is unchanged, the default implementation will suffice
}

A Visitor Implementation:

public class DocProcessor implements DocVisitor<Result> {
    @Override
    private Result processDocGeneric(Result result){
        result.setField1("value1");
        result.setField2("value2");
        result.setField3("value3");
        return result;
    }
    @Override
    private Result processDoc1(Result result){
        result.setField1("VALUE1");
        return result;
    }
    @Override
    private Result processDoc2(Result result){
        result.setField2("V-A-L-U-E-2");
        return result;
    }
}

Usage:

public static final main(String[] args){
     List<Result> results = // Obtain results somehow
     DocProcessor processor = new DocProcessor();
     for(Result result: results){
         processor.visit(result);
     }
}

---

[How to] split this huge class and make it scalable (since there would be new document types to be added in the future

What I've done is merely to split Document data on Result class / Document Processing on DocProcessor class. If you have other processing that differ from type to type, and which can be extracted to an external class (no need for private field handling, private methods calling etc.), this framework os completely applicable.

If not, you should REALLY consider refactoring it to use polymophism! Make each Document type its own object. Use a strong abstract class to link them all, and if you have many methods that are shared accross several but not all types, then make sub-types accordingly - or use default methods! Java8 FTW

Answer 3

For this situation is applicable builder pattern.

/**
* 
* Hero, the class with many parameters.
* 
*/
public final class Hero {

private final Profession profession;
private final String name;
private final HairType hairType;
private final HairColor hairColor;
private final Armor armor;
private final Weapon weapon;

private Hero(Builder builder) {
  this.profession = builder.profession;
  this.name = builder.name;
  this.hairColor = builder.hairColor;
  this.hairType = builder.hairType;
  this.weapon = builder.weapon;
  this.armor = builder.armor;
}

public Profession getProfession() {
  return profession;
}

public String getName() {
  return name;
}

public HairType getHairType() {
  return hairType;
}

public HairColor getHairColor() {
  return hairColor;
}

public Armor getArmor() {
  return armor;
}

public Weapon getWeapon() {
  return weapon;
}

@Override
public String toString() {

  StringBuilder sb = new StringBuilder();
  sb.append("This is a ")
          .append(profession)
          .append(" named ")
          .append(name);
  if (hairColor != null || hairType != null) {
    sb.append(" with ");
    if (hairColor != null) {
      sb.append(hairColor).append(' ');
    }
    if (hairType != null) {
      sb.append(hairType).append(' ');
    }
    sb.append(hairType != HairType.BALD ? "hair" : "head");
  }
  if (armor != null) {
    sb.append(" wearing ").append(armor);
  }
  if (weapon != null) {
    sb.append(" and wielding a ").append(weapon);
  }
  sb.append('.');
  return sb.toString();
}

/**
 * 
 * The builder class.
 * 
 */
public static class Builder {

  private final Profession profession;
  private final String name;
  private HairType hairType;
  private HairColor hairColor;
  private Armor armor;
  private Weapon weapon;

  /**
   * Constructor
   */
  public Builder(Profession profession, String name) {
    if (profession == null || name == null) {
      throw new IllegalArgumentException("profession and name can not be null");
    }
    this.profession = profession;
    this.name = name;
  }

  public Builder withHairType(HairType hairType) {
    this.hairType = hairType;
    return this;
  }

  public Builder withHairColor(HairColor hairColor) {
    this.hairColor = hairColor;
    return this;
  }

  public Builder withArmor(Armor armor) {
    this.armor = armor;
    return this;
  }

  public Builder withWeapon(Weapon weapon) {
    this.weapon = weapon;
    return this;
  }

  public Hero build() {
    return new Hero(this);
  }
}
}