Title: Mutate? or Create?

"Who is the audience?" - is a key question to consider for any piece of writing and one that programmers often forget to address. Just because the language is C++, Java or Cobol doesn't relieve the author of the responsibility to consider the audience. The code that I write to explore an idea for myself will look very different to the code I write to illustrate the idea in an article and the code that uses the idea in a production environment will look different to both of these.

It is interesting to hear some of the answers programmers give when asked to describe the audience for whom they are writing. The most common answer is to deny that the question is meaningful. Other answers include "there isn't one" and "the compiler". And, to judge from most of the code I see, this is the belief in even more cases than the frequency of these answers suggests.

When I write production code the audience splits into two categories: The main audience is those that will use it from their code but there is also a smaller group who will maintain it. (And the audience for this article also writes code for these audiences.)

What follows describes the meeting between a typical developer and an audience that will be making use of it. The context of this meeting is a review of the public interfaces to the classes in a package prior to implementing them. Present at the review are George (the author), Alice (the system designer) and Harris (another programmer on the project). While this is a true account I have coloured the events and changed the names.

George, Alice and Harris filed nervously into the room. This was the first review of "class designs" and none of them knew what to expect. George had followed my instructions and supplied Javadoc documentation of the package containing the classes being reviewed. Everyone had a printed copy.

I recited the prayer customary on these occasions: "we are here to help George improve his design, to identify issues he needs to consider more fully; we are not here to score points or to do the design ourselves."

The classes to be reviewed related to the manipulation of measures that have values, units and dimensions (e.g. [100, metres, length] or [5, kilos, weight]). Much can be (and has been) said about combining disparate units and commensurate dimensions. It has been said many times, by many people and in many places. I'm not going to repeat it here! Instead, I want to focus on the discussion of these two methods in the Value class:

  class Value implements Cloneable {
      public static Value add(Value a, Value b)
      public Value add(Value v)
      ...
  }

I have deliberately given the methods in this form - and not the Javadocs that were reviewed - because I want you, the reader, to think about the semantics you would expect from these methods before reading on.

This link between the name and the semantics is where considering the audience becomes important. A compiler is not critical of the choice of names - it doesn't care if a method called 'add' is used to print a sequence of Goldbach prime pairs. A programmer, however, might express surprise - even if there was documentation to that effect.

Alice had her opinion of what the add methods should do and, when we got to the description of one of these methods, asserted that it "didn't do what one would expect". I hope you have formed your own expectation of what these methods do. How does it correspond to the following (the documented) behaviour?

  /**
   * returns a new Value containing a
   * quantity that is the sum of the
   * quantities held in 'a' and 'b'
   */
  public static Value add(Value a, Value b)

  /**
   * returns a new Value containing a
   * quantity that is the sum of those held
   * in the current object and that supplied
   */
  public Value add(Value v)

Alice was not in disagreement about the first of these but the second led to a lot of discussion. (In the end the description, the signature and the name of the method all came in for criticism - but we are getting ahead of the story.) To illustrate her point Alice took possession of the whiteboard and produced the following code fragment:

  Value accumulate(Iterator i) {
      final Value total = new Value();
      while (i.hasNext()) {
          total.add((Value)i.next());
      }
      return total;
  }

(Actually, Alice's code used shorter identifiers "V" for "Value", "t" for "total", but she and I are writing in a different context and code differently as a consequence.)

She claimed that this code has surprising behaviour. George responded by asserting that "one shouldn't use a method without referring to the documentation". Harris was in full agreement "I don't care what its called I can see what it does".

George took over the whiteboard and suggested the 'correct' code:

  Value accumulate(Iterator i) {
      Value total = new Value();
      while (i.hasNext()) {
          total = total.add((Value)i.next());
      }
      return total;
  }

I think that it is significant that no one queried what Alice thought the original code should achieve. In claiming that it was wrong George and Harris are missing the point - the documentation of a method should support the name given, not invalidate it. Alice suggested that the description should read "adds the supplied value to the current value" or "adds the supplied value to the current value and returns this".

George's correction leads to code that has the intended effect but Harris observed that it creates numerous temporary Values. This leads to the title question:

Should add mutate a Value instance? Or create a new Value? With Alice's change the user has the option of both behaviours. Mutate:

  total.add((Value)i.next());

or create:

  total = Value.add(total,
  (Value)i.next());

George accepted that the suggested behaviour would be useful, but didn't think that the mutating behaviour was reflected by the name add - if you calculate "1 add 2" then neither of the numbers changes to "3".

Harris then suggested that the return type of this add method should be void. His suggestion that the return type should be void came from the desire to invalidate total = total.add((Value)i.next());. This seemed reasonable (but we'll revisit this after hearing what George had been thinking about name of the method).

At this point the meeting moved on to other matters. George took the issues raised away to consider and later circulated a proposal that resolved them.

After drawing an analogy with the + and += operators George suggested that the correct name would be plusEquals. Accepting this argument implies that the original static method should also be renamed plus. In the light of this Harris is concerned about the analogue of "i = i += 1;". This is legal (but clearly daft) for the primitive types and so, by analogy, "total = total.plusEquals((Value)i.next());" should also be legal.

Consequently, George proposed (and ultimately implemented) the following:

  /**
   * returns a new Value containing a
   * quantity that is the sum of the
   * quantities held in 'a' and 'b'
   */
  public static Value plus(Value a, Value b)

  /**
   * Adds the supplied value to that
   * contained in the current object.
   * @return this
   */
  public Value plusEquals(Value v)

The discussion above took about 15 minutes and involved four developers. It avoided any developer that used Value writing code like Alice's example and subsequently spending an indeterminate amount of time looking for the problem.

The fact that the review forced documentation to be written for the add method also prevented someone writing code like Alice's finding the problem and then "correcting" the add method to the detriment of any code that used it as intended.

Are these changes important? Considered from the compiler's point of view it makes little difference. From the available functionality it makes little difference. From the point of view of providing an unsurprising interface that is easy to use effectively it does make a difference.