The first ‘Clean Code’ meeting led to a good productive discussion, so I guess I’m writing more of these. We’ve now moved on to Chapter 3 of Robert C. Martin’s book Clean Code¹, which is all about functions. As before, I’ll be basing my thoughts around the main discussion point for this meeting:

What 2 items in this chapter stood out to you?

There were so many opinionated comments in this chapter that I couldn’t settle for just the two. So I’ve written down some quotes that bothered me, for some reason or another.

Functions should do one thing. They should do it well. They should do it only.

While I agree with this, there’s so much difficulty in determining what “one thing” means that I think the real secret here isn’t in this quote at all. I think the real meat of the message here is what Martin says about abstraction.

One Level of Abstraction per Function.

I think the abstraction comment is much more important. At a high level, your main function will effectively run everything in your application. Obviously that’s expressed as a single function, e.g. showStartScreen(), and then there’s a chain of stuff down. But it’s still doing multiple ‘things’. It’s just at the level of abstraction from main, there is one thing which is showStartScreen().

I think this point is presented well enough in the book, but perhaps with too much emphasis on the “do one thing” component of it. I don’t know if it’s clear enough, even with the examples given, that “one thing” only makes sense at a given level of abstraction.

We want code to read like a top-down narrative.

I agree with this, and I was expecting to see more on the structuring or ordering of functions within a class. Depending on your language, functions don’t all have the same access modifiers. So you’ll have public functions, which you show to the rest of the world, and private functions, which are for internal use only.

I strongly prefer to have all my public functions at the top of the class, because they are providing a narrative to the broader world about what this class can do (or what you can do with this class). The private methods are not part of that narrative; they are part of an inner narrative. I consider the first narrative more important. It’s very rare that I would permit a private function to exist between two public ones. To my mind, that would be a bit like having footnotes or citations in the middle of your text, rather than presented at the end. You look them up when you need that information.

The ideal number of arguments for a function is zero.

Martin makes a strong case for member variables, but that can often lead to classes being more ‘stateful’. It’s amusing to me that Functional Programming advocates for ‘cleaner’ functions that operate only on their input to avoid any sort of mutable state. I think Martin is thinking purely from the perspective of calling a function, rather than reading or writing the function itself.

In the latter case, I think there is a very strong argument to be made in support of having all the inputs provided to your function and not having to worry about state. In doing so you can often limit or eliminate concerns about side-effects and thread safety. I don’t really think Martin makes a case for this at all. It’s my opinion that state should not be added to make functions have fewer arguments.

Output arguments are harder to understand than input arguments.

On this I totally agree. We still find odd places in Java and Android where the API is written like this, sometimes even places that don’t return a value (but could). Sometimes it’s done for efficiency’s sake (for example, to reuse arrays in drawing code that might otherwise be rebuilt after every frame is drawn) but even in those cases it’s unpleasant and confusing to come across such a thing.

One of my first programming languages was Ada 95. It declared methods that looked like this:²

-- Remove from the stack.
procedure Pop(S: in out Int_Stack; I: out Integer) is
   Old_Top: Node_Ptr;
   begin
      if S /= null then
         Old_Top := Node_Ptr(S);
         S := Int_Stack(S.Next);
         I := Old_Top.Data;
         Free(Old_Top);
      end if;
   end Pop;

Note that the arguments list above includes a Stack that gets passed in and out, and a second argument that only gets passed out (equivalent to a return). In Ada 95, you could declare these in any order and flag them as being input or output parameters.

When I first started learning this seemed really intuitive. It was quite hard for me at times to move to Java and discover that I couldn’t, for example, return x and y coordinates out of a single function. I can now attribute that to me growing as a developer, and understanding better when to decompose a function, when to add data objects, and so on.³

When a function seems to need more than two or three arguments, it is likely that some of those arguments ought to be wrapped in a class of their own.

I broadly agree with this, but on the other hand I’m not going to create data objects for the express purpose of passing fewer arguments to a function. Such objects need to serve a purpose on their own, or group data that should only ever be grouped together.

Prefer Exceptions to Returning Error Codes.

This seems obvious, but I think that the general short-hand of -1 and null as ‘signifiers of failure’ is acceptable in cases where an Exception would be an expensive operation. Exceptions should certainly be used for error handling, but never for general flow of control, in my opinion (at least in the languages that I work in).

For example, if querying a database, I’d rather we returned null for a value that doesn’t exist rather than throw some sort of EntryNotFoundException. Yes, the latter can force handling of this error case, but:

• It’s expensive to build a stack trace which is done whenever an exception is thrown.⁴
• If we’re throwing a RuntimeException as part of flow of control then it’s nothing more than a blind GOTO.

If we expect this to be a regular thing that happens then throwing exceptions should be avoided in favour of well-defined and broadly understood values such as null.

Extract Try/Catch Blocks

The example provided splits up an operation into two parts, one of which attempts to do an operation and another that calls the first and handles any exception thrown. I think it’s ironic given the subject matter that the names don’t really make it clear which method does which job; one is called delete() and the other is called deletePageAndAllReferences(). Yes, one method has a throws declaration for the Exception, which the other does not have. Going by names alone however, it’s not obvious what the split of responsibility is here.

I’m not convinced at all that it makes sense to separate the error handling for some event from the code that initiates the event if that error handling is localised (as it is here). In my opinion this just makes the code harder to read. We’re either throwing the exception out into the world for our caller to deal with, or handling it. Cutting this down the middle while chasing a smaller function line count seems like a quick way to make a mess. I also think there’s a discussion here to be had about the levels of abstraction in play.

That said, I do sometimes write code like this, especially for long-running IO operations, or when multiple calls might throw an Exception. So it’s possible I’m just arguing against a bad example.

Writing software is like any other kind of writing.

I think throughout this chapter there’s an underlying thread about code readability. Martin here is talking about writing the ‘first draft’ as you would any other code, and then refining it into something pure and clean. I don’t think the ‘refinement’ stage happens in practice as often as it should.

Master programmers think of systems as stories to be told rather than programs to be written.

This is something that’s been sitting at the back of my mind the whole time, but I haven’t seen it expressed in quite this way. I have certainly used the word ‘narrative’ to try to explain why a class should be written a certain way. It’s why I focus on putting the public components of a class together, at the top, and why I obsess about things like naming. I think this idea of code telling a story is a crucial part of code readability and the concept of ‘clean code’ in general.