Separated Presentation

Example: Moving Domain Logic out of a Window (Java)

Most of examples you'll see from me follow Separated Presentation, simply because I find it such a fundamental design technique. Here's an example of how you can refactor a simple design that doesn't use Separated Presentation in order to use it.

The example comes from the running example of Ice Cream Atmospheric Monitor. The main task I use to illustrate this is calculating the variance between target and actual and coloring the field to indicate the amount of this variance. You can imagine this done in the assessment window object like this:

class AssessmentWindow...

  private JFormattedTextField dateField, actualField, targetField,
  varianceField;
  Reading currentReading;

   private void updateVarianceField() {
       if (null == currentReading.getActual()) {
           varianceField.setValue(null);
           varianceField.setForeground(Color.BLACK);
       }
       else {
           long variance = currentReading.getActual() - currentReading.getTarget();
           varianceField.setValue(variance);
           long varianceRatio = Math.round(100.0 * variance / currentReading.getTarget());
            if (varianceRatio < -10) varianceField.setForeground(Color.RED);
            else if (varianceRatio > 5) varianceField.setForeground(Color.GREEN);
            else varianceField.setForeground(Color.BLACK);
        }
   }

As you can see this routine mixes the domain problem of calculating the variance with the behavior of updating the the variance text field. The Reading object, that holds the actual and target data, is here a data class - an anemic collection of fields and accessors. Since this is the object that has the data, it should be the one to calculate the variance.

To start this I can use Replace Temp with Query on the variance calculation itself, to yield this.

class AssessmentWindow...

  private void updateVarianceField() {
       if (null == currentReading.getActual()) {
           varianceField.setValue(null);
           varianceField.setForeground(Color.BLACK);
       }
       else {
           varianceField.setValue(getVariance());
           long varianceRatio = Math.round(100.0 * getVariance() / currentReading.getTarget());
            if (varianceRatio < -10) varianceField.setForeground(Color.RED);
            else if (varianceRatio > 5) varianceField.setForeground(Color.GREEN);
            else varianceField.setForeground(Color.BLACK);
        }
   }
   private long getVariance() {
       return currentReading.getActual() - currentReading.getTarget();
   }

With the calculation now in its own method, I can safely move it to the Reading object.

class AssessmentWindow...
   private void updateVarianceField() {
        if (null == currentReading.getActual()) {
            varianceField.setValue(null);
            varianceField.setForeground(Color.BLACK);
        }
        else {
            varianceField.setValue(currentReading.getVariance());
            long varianceRatio = Math.round(100.0 * currentReading.getVariance() / currentReading.getTarget());
             if (varianceRatio < -10) varianceField.setForeground(Color.RED);
             else if (varianceRatio > 5) varianceField.setForeground(Color.GREEN);
             else varianceField.setForeground(Color.BLACK);
         }
    }
class Reading...
    public long getVariance() {
        return getActual() - getTarget();
    }

I can do the same thing with the varianceRatio, I'll just show the final result but again I do it to steps (creating the local method then moving it) as I'm less likely to mess it up that way - particularly with the refactoring editor (IntelliJ Idea) I'm using.

class AssessmentWindow...
   private void updateVarianceField() {
        if (null == currentReading.getActual()) {
            varianceField.setValue(null);
            varianceField.setForeground(Color.BLACK);
        }
        else {
            varianceField.setValue(currentReading.getVariance());
            if (currentReading.getVarianceRatio() < -10) varianceField.setForeground(Color.RED);
            else if (currentReading.getVarianceRatio() > 5) varianceField.setForeground(Color.GREEN);
            else varianceField.setForeground(Color.BLACK);
         }
    }

class Reading...
   public long getVarianceRatio() {
        return Math.round(100.0 * getVariance() / getTarget());
    }

The calculations are looking better, but I'm still not too happy. The logic about when the color should be one color or another is domain logic, even though the choice of color (and the fact that text color is the presentation mechanism) is presentation logic. What I need to do is split the out the determination of which category of variance we have (and the logic for assigning these categories) from the coloring.

There's no formalized refactoring here, but what I need is a method like this on Reading.

class Reading...
    public enum VarianceCategory {LOW, NORMAL, HIGH}

    public VarianceCategory getVarianceCategory() {
         if (getVarianceRatio() < -10) return VarianceCategory.LOW;
         else if (getVarianceRatio() > 5) return VarianceCategory.HIGH;
         else return VarianceCategory.NORMAL;
    }

class AssessmentWindow...
    private void updateVarianceField() {
        if (null == currentReading.getActual()) {
            varianceField.setValue(null);
            varianceField.setForeground(Color.BLACK);
        }
        else {
            varianceField.setValue(currentReading.getVariance());
            if (currentReading.getVarianceCategory() == Reading.VarianceCategory.LOW) varianceField.setForeground(Color.RED);
            else if (currentReading.getVarianceCategory() == Reading.VarianceCategory.HIGH) varianceField.setForeground(Color.GREEN);
            else varianceField.setForeground(Color.BLACK);
         }
    }

That's better, I now have the domain decisions in the domain object. But things are a bit messy, the presentation shouldn't have to know that the variance is null if the actual reading is null. That kind of dependency should be encapsulated in the Reading class.

class Reading...
   public Long getVariance() {
        if (null == getActual()) return null;
        return getActual() - getTarget();
    }

class AssessmentWindow...
    private void updateVarianceField() {
        varianceField.setValue(currentReading.getVariance());
        if (null == currentReading.getVariance()) {
            varianceField.setForeground(Color.BLACK);
        }
        else {
            if (currentReading.getVarianceCategory() == Reading.VarianceCategory.LOW) varianceField.setForeground(Color.RED);
            else if (currentReading.getVarianceCategory() == Reading.VarianceCategory.HIGH) varianceField.setForeground(Color.GREEN);
            else varianceField.setForeground(Color.BLACK);
         }
    }

I can further encapsultate this by adding a null variance category, which also allows me to use a switch that I find easier to read.

class Reading...
  public enum VarianceCategory {
        LOW, NORMAL, HIGH, NULL}

    public VarianceCategory getVarianceCategory() {
        if (null == getVariance()) return VarianceCategory.NULL;
        if (getVarianceRatio() < -10) return VarianceCategory.LOW;
        else if (getVarianceRatio() > 5) return VarianceCategory.HIGH;
        else return VarianceCategory.NORMAL;
    }

class AssessmentWindow...
   private void updateVarianceField() {
        varianceField.setValue(currentReading.getVariance());
        switch (currentReading.getVarianceCategory()) {
            case LOW:
                varianceField.setForeground(Color.RED);
                break;
            case HIGH:
                varianceField.setForeground(Color.GREEN);
                break;
            case NULL:
                varianceField.setForeground(Color.BLACK);
                break;
            case NORMAL:
                varianceField.setForeground(Color.BLACK);
                break;
            default:
                throw new IllegalArgumentException("Unknown variance category");
        }
     }

As a last step, although not connected to Separated Presentation, I prefer to clean that switch up to remove the duplication.

class AssessmentWindow...
  private void updateVarianceField() {
        varianceField.setValue(currentReading.getVariance());
        varianceField.setForeground(varianceColor());
    }

    private Color varianceColor() {
        switch (currentReading.getVarianceCategory()) {
            case LOW:
                return Color.RED;
            case HIGH:
                return Color.GREEN;
            case NULL:
                return Color.BLACK;
            case NORMAL:
                return Color.BLACK;
            default:
                throw new IllegalArgumentException("Unknown variance category");
        }
    }

This makes it obvious that what we have here is a simple table lookup. I could replace this by populating and indexing out of a hash. In some languages I might do that, but I find the switch here to be nice and readable in Java.