Thinking in OOP way

Question

Whenever I think that I am gaining some confidence in OOP then suddenly I get bitten by some advance example. Like in this very great article by Uncle Bob he uses the below class an example for his kata.

 public class WordWrapper {
      private int length;

  public WordWrapper(int length) {
    this.length = length;
  }

  public static String wrap(String s, int length) {
    return new WordWrapper(length).wrap(s);
  }

  public String wrap(String s) {
    if (length < 1)
      throw new InvalidArgument();
    if (s == null)
      return "";

    if (s.length() <= length)
      return s;
    else {
      int space = s.indexOf(" ");
      if (space >= 0) 
        return breakBetween(s, space, space + 1);
      else
        return breakBetween(s, length, length);
    }
  }

  private String breakBetween(String s, int start, int end) {
    return s.substring(0, start) + 
      "\n" + 
      wrap(s.substring(end), length);
  }

  public static class InvalidArgument extends RuntimeException {
  }
}

I have following doubts:

1. Why the static helper method wrap?
2. Why the InvalidArgument class is nested and static?
3. Why do we even need to initialize this class since its nothing but an algorithm and can operate without any instance variable, why we need ~100 instances(for eg) of it?

Answer 1

1. Why the static helper method wrap?

There is no especially good reason - I think that it is a subjective judgement that:

WordWrapper.wrap("foo", 5);

is neater than

new WordWrapper(5).wrap("foo");

(which I would agree it is). I tend to find myself adding methods like this when the code just feels very repetitive.

However, the static form can lead to hidden problems: invoking that in a loop results in the creation of a lot of unnecessary instances of WordWrapper, whereas the non-static form just creates one and reuses it.

2. Why the InvalidArgument class is nested and static?

The implication of it being nested is that it is only for use in reporting invalid arguments of methods in WordWrapper. For instance, it wouldn't make much sense if some database-related class threw an instance of WordWrapper.InvalidArgument.

Remember that you can reference it as InvalidArgument for convenience if appropriately imported; you're still always using some.packagename.WordWrapper.InvalidArgument, so its use in other classes doesn't make semantic sense.

If you expect to use it in other classes, it should not be nested.

As for why static: there are two reasons that I can think of (which are sort of different sides of the same coin):

1. It doesn't need to be non-static. A non-static nested class is called an inner class. It is related to the instance of the containing class which created it; in some way, the data in the inner class is related to the data in the outer class.

   What this actually means is there is a hidden reference to the outer class passed into the inner class when it is created. If you never need to refer to this instance, make it static, so the reference isn't passed. It's just like removing unused parameters of methods: if you don't need it, don't pass it.

2. Holding this reference has unexpected consequences. (I draw this as a separate point because whereas the previous one refers to a logical requirement/design for the reference or not, this refers to practical implications of holding that reference).

   Just as with holding any reference, if you have a reference to an instance of the inner class, you make everything that it references ineligible for garbage collection, since it is still reachable. Depending upon how you use instances of the inner class, this can lead to a memory leak. The static version of the class doesn't suffer from this problem, since there is no reference: you can have a reference to a InvalidArgument when all of the instances of Wrapper are cleared up.

   Another consequence is that the contract of InvalidArgument is invalid: Throwable, a superclass of InvalidArgument, implements Serializable, meaning that InvalidArgument also implements Serializable. However, WordWrapper is not Serializable. As such, serialization of a non-static InvalidArgument would fail because of the non-null reference to WordWrapper.

   The simple solution to both of these issues is to make the nested class static; as a defensive strategy, one should make all nested classes static, unless you really need them not to be.

3. Why do we even need to initialize this class since its nothing but an algorithm...

Good question. This is sort of related to your first question: you could get away with just the static helper method, and remove the instance methods and state.

Before you chuck away your instance methods, there are advantages to instance methods over static methods.

The obvious one is that you are able to store state in the instances, for instance length. This allows you to pass fewer parameters to wrap, which might make the code less repetitive; I suppose it gives an effect a bit like partial evaluation. (You can store state in static variables too, but global mutable state is a royal PITA; that's another story).

Static methods are a tight coupling: the class using WordWrapper is tightly bound to a specific implementation of word wrapping.

For many purposes, one implementation might be fine. However, there is almost always a case for at least two implementations (your production and test implementations).

So, whereas the following is tightly bound to one implementation:

void doStuffWithAString(String s) {
  // Do something....
  WordWrapper.wrap(s, 100);
  // Do something else ....
}

the following can have an implementation provided at runtime:

void doStuffWithAString(WordWrapper wrapper, String s) {
  // Do something....
  wrapper.wrap(s);
  // Do something else ....
}

which is using the wrapper as a strategy.

Now, you can select the word wrapping algorithm used for a particular case (e.g. one algorithm works well for English, but another works better for Chinese - maybe, I don't know, it's just an example).

Or, for a test, you can inject a mocked instance for tests which just returns the parameter - this allows you to test doStuffWithAString without testing the implementation of WordWrapper at the same time.

But, with flexibility comes overhead. The static method is more concise. For very simple methods, static could well be the way to go; as the method gets more complicated (and, particularly in the testing case, it becomes harder and harder to work out the input to provide to get a specific output which is important to your test case), the instance method form becomes a better choice.

Ultimately, there is no hard-and-fast rule for which to use. Be aware of both, and notice which works best in given situations.