Keep Your Code DRY

Summary

Don’t Repeat Yourself (DRY) - avoid writing the same logic more than once.

Every time you copy and paste code, flick yourself in the eye. This is a great disincentive to doing it again but over time may cause blindness.

Details

If the same logic is required more than once then it should not be duplicated; it should instead, be extracted to a well named class or method.

This will be both easier to read and easier to maintain because a change will only be required in one place should the logic need to change.

Bad

  1. class Foo {
  2.    private int status;
  3.    private boolean approved;
  5.    foo() {
  6.      if (status == 12 || approved) {
  7.        doFoo();
  8.      }
  9.    }
  11.    bar() {
  12.      if (status == 12 || approved) {
  13.        doBar();
  14.      }
  15.    }
  16. }

Better

  1. class Foo {
  2.    private final static int PRE_APPROVED = 12;
  4.    private int status;
  5.    private boolean approved;
  7.    foo() {
  8.      if (isApproved()) {
  9.        doFoo();
  10.      }
  11.    }
  13.    bar() {
  14.      if (isApproved()) {
  15.        doBar();
  16.      }
  17.    }
  19.    private isApproved() {
  20.      return status == PRE_APPROVED || approved;
  21.    }
  22. }

Things are a little trickier when we have similar but not identical logic.

Although it is quick and easy, the worst thing we can do is copy and paste.

Terrible

  1. public void doSomething(List<Widget> widgets) {
  2.   for (Widget widget : widgets) {
  3.     reportExistence(widget);
  4.     if (widget.snortles() > 0) {
  5.       reportDeviance(widget);
  6.       performSideEffect(widget);
  7.     }
  8.   }
  9. }
  11. public void doSomethingSimilar(List<Widget> widgets) {
  12.   for (Widget widget : widgets) {
  13.     reportExistence(widget);
  14.     if (widget.snortles() > 0) {
  15.       reportDeviance(widget);
  16.       performDifferentSideEffect(widget);
  17.     }
  18.   }
  19. }

This seemed quick and easy now, but is the start of a codebase that will suck time each time we try to understand or change it.

A straightforward but very limited approach to re-use code is to introduce Boolean flags.

Not great

  1. public void doSomething(List<Widget> widgets) {
  2.   doThings(widgets, false);
  3. }
  5. public void doSomethingSimilar(List<Widget> widgets) {
  6.   doThings(widgets, true);
  7. }
  10. private void doThings(List<Widget> widgets, boolean doDifferentSideEffect) {
  11.   for (Widget widget : widgets) {
  12.     reportExistence(widget);
  13.     if (widget.snortles() > 0) {
  14.       reportDeviance(widget);
  15.       if (doDifferentSideEffect) {
  16.         performDifferentSideEffect(widget);
  17.       } else {
  18.         performSideEffect(widget);
  19.       }
  20.     }
  21.   }
  22. }

This is ugly and gets worse as the number of possibilities increases.

A much more scalable approach is to use the Strategy pattern.

If we introduce an interface:

  1. interface WidgetAction {
  2.    void apply(Widget widget);
  3. }

Then we can use it as follows:

Better

  1. public void doSomething(List<Widget> widgets) {
  2.   doThings(widgets, performSideEffect());
  3. }
  6. public void doSomethingSimilar(List<Widget> widgets) {
  7.   doThings(widgets, performDifferentSideEffect());
  8. }
  10. private WidgetAction performSideEffect() {
  11.   return new WidgetAction() {
  12.     @Override
  13.     public void apply(Widget widget) {
  14.       performSideEffect(widget);
  15.     }
  16.   };
  17. }
  19. private WidgetAction performDifferentSideEffect() {
  20.   return new WidgetAction() {
  21.     @Override
  22.     public void apply(Widget widget) {
  23.       performDifferentSideEffect(widget);
  24.     }
  25.   };
  26. }
  28. private void doThings(List<Widget> widgets, WidgetAction action ) {
  29.   for (Widget widget : widgets) {
  30.     reportExistence(widget);
  31.     if (widget.snortles() > 0) {
  32.       reportDeviance(widget);
  33.       action.apply(widget);
  34.     }
  35.   }
  36. }

The Java 7 version is quite verbose due to the anonymous inner class boiler plate.

Arguably, Boolean flags might be preferable for very simple cases such as this but, if we extract the logic in performSideEffect and performDifferentSideEffect methods into top-level classes implementing WidgetAction, then the Strategy version becomes compelling.

In Java 8, there is little question that the Strategy pattern is preferable in even the simplest of cases.

Better with Java 8

  1. public void doSomething(List<Widget> widgets) {
  2.   doThings(widgets, widget -> performSideEffect(widget));
  3. }
  5. public void doSomethingSimilar(List<Widget> widgets) {
  6.   doThings(widgets, widget -> performDifferentSideEffect(widget));
  7. }
  9. private void doThings(List<Widget> widgets, Consumer<Widget> action ) {
  10.   for (Widget widget : widgets) {
  11.     reportExistence(widget);
  12.     if (widget.snortles() > 0) {
  13.       reportDeviance(widget);
  14.       action.accept(widget);
  15.     }
  16.   }
  17. }

We do not need to introduce our own interface - the built-in Consumer<T> is enough. We should consider introducing one if the doThings method were exposed publicly or if the logic in performSideEffect was complex enough to pull into a top-level class.

The loop might also be converted to a pipeline.

As a pipeline

  1. private void doThings(List<Widget> widgets, Consumer<Widget> action ) {
  2.   widgets
  3.   .stream()
  4.   .peek(widget -> reportExistence(widget))
  5.   .filter(widget -> widget.snortles() > 0)
  6.   .peek(widget -> reportDeviance(widget))
  7.   .forEach(action);
  8. }