Clean Code – Writing Small Functions vs Doing Everything in the Same Loop

clean codedesign-patternsfunctionsrefactoring

I have read a book called Clean Code by Robert C. Martin. In this book I've seen many methods to clean up code like writing small functions, choosing names carefully, etc. It seems by far the most interesting book about clean code I've read. However, today my boss didn't like the way I wrote code after reading this book.

His arguments were

  • Writing small functions is a pain because it forces you to move into each small function to see what the code is doing.
  • Put everything in a main big loop even if the main loop is more than 300 lines, it is faster to read.
  • Only write small functions if you have to duplicate code.
  • Don't write a function with the name of the comment, put your complex line of code (3-4 lines) with a comment above; similarly you can modify the failing code directly

This is against everything I've read. How do you usually write code? One main big loop, no small functions?

The language I use is mainly Javascript. I really have difficulties reading now since I've deleted all my small clearly named functions and put everything in a big loop. However, my boss likes it this way.

One example was:

// The way I would write it
if (isApplicationInProduction(headers)) {
  phoneNumber = headers.resourceId;
} else {
  phoneNumber = DEV_PHONE_NUMBER;
}

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

// The way he would write it
// Take the right resourceId if application is in production
phoneNumber = headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;

In the book I've read for example comments are considered as failure to write clean code because they are obsolete if you write small functions and often leads to non-updated comments (you modify your code and not the comment). However what I do is delete the comment and write a function with the name of the comment.

Well, I would like some advice, which way/practice is better to write clean code?

Best Answer

Taking the code examples first. You favour:

if (isApplicationInProduction(headers)) {
  phoneNumber = headers.resourceId;
} else {
  phoneNumber = DEV_PHONE_NUMBER;
}

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

And your boss would write it as:

// Take the right resourceId if application is in production
phoneNumber = headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;

In my view, both have problems. As I read your code, my immediate thought was "you can replace that if with a ternary expression". Then I read your boss' code and thought "why's he replaced your function with a comment?".

I'd suggest the optimal code is between the two:

phoneNumber = isApplicationInProduction(headers) ? headers.resourceId : DEV_PHONE_NUMBER;

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

That gives you the best of both worlds: a simplified test expression and the comment is replaced with testable code.

Regarding your boss' views on code design though:

Writing small functions is a pain because it forces you to move into each small functions to see what the code is doing.

If the function is well-named, this isn't the case. isApplicationInProduction is self-evident and it should not be necessary to examine the code to see what it does. In fact the opposite is true: examining the code reveals less as to the intention than the function name does (which is why your boss has to resort to comments).

Put everything in a main big loop even if the main loop is more than 300 lines, it is faster to read

It may be faster to scan through, but to truly "read" the code, you need to be able to effectively execute it in your head. That's easy with small functions and is really, really hard with methods that are 100's of lines long.

Write only small functions if you have to duplicate code

I disagree. As your code example shows, small, well-named functions improve readability of code and should be used whenever eg you aren't interested in the "how", only the "what" of a piece of functionality.

Don't write a function with the name of the comment, put your complex line of code (3-4 lines) with a comment above. Like this you can modify the failing code directly

I really can't understand the reasoning behind this one, assuming it really is serious. It's the sort of thing I'd expect to see written in parody by The Expert Beginner twitter account. Comments have a fundamental flaw: they aren't compiled/interpreted and so can't be unit tested. The code gets modified and the comment gets left alone and you end up not knowing which is right.

Writing self-documenting code is hard, and supplementary docs (even in the form of comments) are sometimes needed. But "Uncle Bob"'s view that comments are a coding failure holds true all too often.

Get your boss to read the Clean Code book and try to resist making your code less readable just to satisfy him. Ultimately though, if you can't persuade him to change, you have to either fall in line or find a new boss that can code better.