Improving The Coffee Mug Code

Good code reads like English. As much as possible anyway. For Christmas, Santa brought me a coffee mug, and it has printed on its side a short, hilarious code snippet that approaches this ideal.

Here is the code again:

  while (working)
  {
     coffee.drink();
     work.execute();
     if (coffee == "empty")
     {
        if (coffeepot == "empty")
           coffeepot.brew();
        coffee.refill(); 
     }
  }

Yes, it’s pretty clear what’s going on.

But could it be even better?

Yesterday I asked you to send in your answers on how you would improve the coffee mug code. And send in your answers you did. I was delighted with the audience participation level: I got three well-thought-out responses.

The first response was from Andrew Baird. Andy suggested replacing the string comparisons in the two if statements with boolean properties (or methods) on the coffee and coffeepot object, respectively:

  while (working)
  {
     coffee.drink();
     work.execute();
     if (coffee.isEmpty)
     {
        if (coffeepot.isEmpty)
           coffeepot.brew();
        coffee.refill(); 
     }
  }

What I like about Andy’s idea is the increase in the code’s abstractness; it is no longer concerned with measuring emptiness via direct string comparison. The exposed isEmpty properties on coffee and coffeepot hide the implementation detail. After all, this is high-level code—it should only be concerned in determining that a vessel is empty (or not), and not at all with how this determination occurs.

Andy also pointed out that the working variable’s boolean state could be absorbed into the work object—maybe as another property, haveWork. Unfortunately, if(work.haveWork), has lost us the joke of the original code.

Natasha’s response mirrored Andy’s regarding bringing the state of the working variable into the work object. Natasha named her property isInProgress, so the predicate becomes if(work.isInProgress), which I slightly prefer to if(work.haveWork)since it reads more like English.

Natasha spotted another improvement, one which had eluded me: The first two lines in the while loop, coffee.drink() and work.execute(), are more effective as the last two lines in the loop. Natasha is right when she says that there is no point refilling the coffeepot after doing our work.

But there is another reason why coffee.drink() and work.execute() should go at the end: It’s safer. Thank you, Eugine, for coming up with this gem. After all, we can’t drink an empty cup of coffee! If coffee is empty to start with then coffee.drink() may throw an exception or otherwise destabilise the program. In any case, without coffee, we would not have the energy to power our work execution, would we?!?

However, if we first check and refill as necessary, then we get to drink our coffee and execute our work:

  while (work.isInProgress)
  {
     if (coffee.isEmpty)
     {
        if (coffeepot.isEmpty)
           coffeepot.brew();
        coffee.refill(); 
     }
     coffee.drink();
     work.execute();
  }

I find it fascinating how we ended up in a situation where we ‘do’ our work on the last line and all prior lines inside the loop are concerned with getting the coffee sorted. Talk about code mimicking real life! ;-)

We are almost done. Now all the coffee preparation code could go into a separate local method:

  private void prepareCoffee(Coffee coffee)
  {
     if (coffee.isEmpty)
     {
        if (coffeepot.isEmpty)
           coffeepot.brew();
        coffee.refill(); 
     }
  }

Finally leaving us with

  while (work.isInProgress)
  {
     prepareCoffee(coffee);
     coffee.drink();
     work.execute();
  }

Not bad!

One more thing. Could we have encapsulated the coffee preparation code on the coffee object itself? Yes, I suppose we could have defined a coffee.prepare() method. Logically, I have my doubts as to whether the coffeepot object should entirely be subsumed, via Aggregation or Composition, into the coffee object. It seems that coffee and coffeepot are more like siblings to one another than coffee owning the behaviour of coffeepot. Therefore it makes more sense, IMHO, to have coffee prepared at the highest level—and so we end up with prepareCoffee(coffee).