Is Clean Code less Code?

For me clean code is simple, easy to understand code. No over-engineering, as little boilerplate as possible and of course non-cryptic. But does that lead to less code?

I really don’t think so. Most of the times less code is more cryptic and harder to understand (and therefore harder to maintain).

When working with jBehave and testing the meta filtering, I wrote something similar to the following code:

@Override
public Embedder configuredEmbedder() {
   Embedder embedder = super.configuredEmbedder();
   ignoreStoriesAndScenariosWithMetaInformationParameter(embedder, "ignore");
   return embedder;
}

private void ignoreStoriesAndScenariosWithMetaInformationParameter(Embedder embedder, String ignoreParameter) {
    embedder.useMetaFilters(Arrays.asList("-" + ignoreParameter));
}

Later, I had a discussion about that code and a colleague said he just deleted the “unnecessary” private method which leads to the following code:

@Override
public Embedder configuredEmbedder() {
   Embedder embedder = super.configuredEmbedder();
   embedder.useMetaFilters(Arrays.asList("-ignore"));
   return embedder;
}

Obviously shorter and less methods. For us, working on that specific class, it may be obvious what’s happening in this method, but just imagine somebody joining the project and never working with jBehave before. For him the longer code provides additional information and even if he does not understand how it works and what “meta filters” are and what the minus means, he’ll at least understand what we are trying to achieve.

When I try to explain my choice, other developers agreed to my point but said that we can also achieve the same result with a comment. And yes, I totally agree that this is a valid option. I think this is just a question of style. I personally don’t like comments (and Uncle Bob’s “Clean Code” is on my site), but maybe in this case the comment really is the better choice because you can explain the connection between this meta filter code and the jBehave story files.

So the resulting code is the following:

@Override
public Embedder configuredEmbedder() {
   Embedder embedder = super.configuredEmbedder();
   // ignore stories and scenarios with meta information parameter @ignore.
   embedder.useMetaFilters(Arrays.asList("-ignore"));
   return embedder;
}

Of course you can argue, that this small example isn’t worth talking about. But I think within a project style it is important. And you can find a common style by discussing specific examples. Maybe the other developer will think about if his code could may be unclear to project joiners and I will think about a comment instead of one-line methods.

Conclusion

So clean code isn’t always less code. You need to trade off the benefits of more small methods against lesser lines of code. I’ll discuss the other examples about coding style in some future blog posts.

Which solution is your favorit one and why?

12 Responses to “Is Clean Code less Code?”

  1. Sam Atkinson says:

    THE ANSWER IS NEVER A COMMENT.

    I know I’m preaching to the clean-code converted choir, but comments like this are terrible. Code will change and refactor, this comment will end up out of date and irrelevant. Clean code is self documenting.

    Ideally you’d be able to edit the name of “useMetaFilters” but I’m guessing this is a library method. As a result, it would be between your version and the inlined one. Generally I think I would go with your version, although I’d try and shorten ignoreStoriesAndScenariosWithMetaInformationParameter if possible. ignoreThingsWithParameter(“-ignore”) perhaps, although I don’t know the context as to whether that would make sense.

    Good job though. Keep fighting the good fight.

  2. Philip says:

    Hey Nina,

    you should find the following talk interesting: Four Patterns at the Heart of Good Programming Style.
    For best results, download it and read the notes accompanying each slide.

    Philip

  3. Waldemar Biller says:

    Hey Nina, the problem with private is that it modifies the state of embedder and hides it away behind a “ignoreStoriesAndScenariosWithMetaInformationParameter”. That’s not so readable 😉

    • I disagree. The method name describes how it modifies the state of the embedder.

      Like @SamAtkinson said, the method name is a bit long, but it is definitely better than putting the line to the configuredEmbedder() method because it requires a comment. The problem of comments is that they aren’t updated when the code is changed.

      The example shown on this blog post is so simple that the benefits of using private method might seem very small, but I have noticed that small improvements can make a huge difference when you combine them with other small improvements.

      Of course, this is probably a matter of preference, and that is why it is a good topic for passionate discussion. 😉

  4. dbpokorny says:

    I just had a marvelous experience with comments in source code; if you check out https://github.com/botwillacceptanything/botwillacceptanything and install node and set it up, then you get a node_modules directory containing node_modules/github/api/v3.0.0/pullRequests.js which has beautiful comments. The other files are similarly spectacular.

    If you want to understand the role of comments in source code at a high level, I would start with Code Complete, in particular the sections on the pseudocode programming process. Here is a blog post that discusses it: http://davidzych.com/writing-code-using-the-pseudocode-programming-process/ Steve also talks a lot about the central role coding conventions play ( http://en.wikipedia.org/wiki/Coding_conventions ). Now we can talk about whether or not a piece of code sufficiently adheres to the conventions of a project and forget about picking favorites and “better” and “worse”.

    There is “better in general” and “better for this particular project because …”. And there are intrinsic reasons (this piece of code uses N variables and has cyclomatic complexity K) and extrinsic reasons (one of the senior developers likes style Y and the other developers have already bought into supporting and defending the choice of Y). There are technical reasons and political reasons…

    A variation on this theme is the use of assertions in code. Microsoft did a study: http://research.microsoft.com/pubs/70290/tr-2006-54.pdf

  5. I don’t have a strong preference in this small example, but I suspect a larger example would favor the method over the comment. I agree with Sam that this method name seems long. If that’s really necessary; sure, but I doesn’t sound like it here.

    One minor adjustment I’d consider is to remove the ignoreParameter parameter because that sounds like premature generality. Removing the parameter decreases the size difference and makes this a little more of an apples-to-apples comparison..

    As a fourth option (in addition to run-on code, commented code, and using a subroutine), you could look at a variable – the name is often as good as a method’s for clarity, but shorter in usage. Unfortunately, for mutating methods this isn’t a good fit. In this example you might name the filter instead of the action of applying it.

  6. thbt says:

    In this example, I agree. Self-documenting code is a beautiful thing when it makes sense. But in this case, it’s better to add a comment rather than create a whole new method that will only ever be called once (with the comment-as-method-name name “ignoreStoriesAndScenariosWithMetaInformationParameter()”).

    Don’t mindlessly follow some dogma like the “THE ANSWER IS NEVER A COMMENT”. Use what makes the most sense in each situation. Strive for self-documenting code when naming functions and variables, but don’t add complexity when a simple comment will suffice.

    Too often programmers think they’re making code easier to maintain and understand by taking a straight-forward 10-line method and breaking it up into a maze of 10 different methods, each of which is only ever called once in the entire project. Sure there aren’t any comments, but it can take an hour to trace through all the method calls to find the nuts and bolts of what the method is actually doing. Rarely is it ever easier to debug, understand, or refactor 10 methods instead of one simple method with some comments for clarity.

    Whenever someone advocates “always” following any programming dogma, take a step back and consider both options and ask yourself which is truly easier to understand and maintain.

  7. u/CodeEverywhere says:

    Good article 🙂 I guess ‘clean’ code doesn’t always mean the fewest lines of code. I personally love cramming as many functions into one line as I can. I’m shying away from that now that I’ve gone over a few old projects of mine and scratched my head at my monstrous one-liner creations.

  8. Joss says:

    The problem, as always, is that naming things is the hardest part of programming.

    People think its algorithms and tricky maths problems… but that stuff is easy peasy compared to coming up with clear and meaningful names for things.

    The balance between clarity and brevity is near impossible to manage.

    I really think that your colleague likely didn’t object to what you were trying to achieve but simply objected to the unwieldy name you gave it. What it lacked in brevity it didn’t really make up for in clarity.

    I am going to make a fool of myself by suggesting an alternative (perhaps just to prove my point about naming things being the hardest part) but how about:

    excludeContentByParameter(embedder, "ignore");

    Having said all that… a kind of meaningful name is still likely better than a comment 🙂

  9. Jon Mithe says:

    I would rather:

    @Override
    public Embedder configuredEmbedder() {
    Embedder embedder = super.configuredEmbedder();
    embedder.useMetaFilters(filterForIgnoreAnnoation());
    return embedder;
    }

    private List filterForIgnoreAnnotation() {
    return Arrays.asList(“-ignore”);
    }

    It keeps the use of embedder clean / as described without hiding anything away. Really the the clean code here is in the filter generation to say what its doing / simplify that.

    The first example you mentioned (ignoreStoriesAndScenariosWithMetaInformationParameter) is not very clean, you have to keep on passing the embedder out and really as Waldemar Biller says, its being hidden away / code is being wrapped up and delegated to.

    Generally I find passing implementations in that manner hints at bad design in the code. Likely will be cases where this may not be true, e.g. multiple methods my have embedors / can reuse that filter method, but often I would guess extraction to a builder/factory class, or some utility driven type class will likely be a cleaner / more understandable + maintainable way of getting re-use through design.

    With the comment, they are always prone to breaking / being lost on refactor and the assumption that the next person coming along will actually read them…

  10. […] a look at Nina Hartmann’s analysis of whether or not Clean Code is Less Code. Would love to hear some thoughts on her […]

  11. Ime says:

    Why you guys think people can remember to change method name but will forget to change comment? That is prejudice. You forget comment, you will forget method name too.

Leave a Reply

Your email address will not be published. Required fields are marked *