Sharing a conversation about clean code

G'day:
I'm being a bit lazy today. And interesting conversation cropped up on the comm log of one of the side projects I'm working on, and I thought you might be interested to read it, and perhaps add your thoughts.

We were discussing how to handle the requirement for a new API function we're adding to our localisation suite, which works similar to dayOfWeekAsString(), except the immediate requirement it to provide a localised "short version" of the day, eg: "Mon", "Tue", etc. The initial suggestion had come in as dayOfWeekShortAsString(). And the conversation started. I am "Mr Green", incidentally. I have made a few edits to clean up spelling and grammar, but I have not removed anything, or changed the context of what was said. I have added some clarification (for the purposes of this article) part-way down.

Topic:
dayOfWeekAsString() / dayOfWeekShortAsString()




Mr Red

Should we make merge them to one function with a new argument "short:boolean"?

Mr Green

It's bad coding practice to add boolean arguments to a function. I recommend using a mask - as with dateFormat() - instead (as that actually reflects what you want to be doing. A boolean is meaningless)

Mr Red

I disagree. IMO It's a bad idea to use string arguments to a function in an untyped language like CFML. A boolean is very meaningful when there are only two options as long as the argument name is descriptive. Then you know that the value is either true or false, not "milliseconds", "millisec", "millis", "milli", "ms", "m", etc. If you use a strongly typed language where the IDE can type it for you then it's a different story.

Mr Blue

Boolean arguments are a code smell. Mr Green is right. "Robert C. Martin's Clean Code Tip #12: Eliminate Boolean Arguments" "avoid boolean parameters" and "Code Smell Metrics" also talks about this.

Mr Red

I still disagree. the examples you cited use a strongly typed language where you can use enums and static variables, which is very different from passing a string value to a function as an argument.

This might be a good rule for Java, but not for CFML and its like (ie: The Application).

[Just to contextualise here, "The Application" has a large and richly featured public API which developers using The Application use to write their own sub-applications. So this conversation is very much from the perspective of adding methods to an API, not simply thinking about stand-alone functions, or an inward-facing system. The code doesn't simply need to make sense to us: it needs to make sense to our client developers too].

Mr Blue

Read the articles again. It's nothing to do with the type system: boolean flags are a code smell in any language.

Mr Red

Of course that it has to do with type system. In a strongly-typed language the IDE and/or compiler will autocomplete/check the values for you so there is no possibility of you to make a mistake like writing getTickCount('nanoseconds') when the correct value should be getTickCount('nanos'), or maybe it is getTickCount('nano')... who knows?

In a typed language you will pass a value that is checked at edit time so that you don't get weird behavior at runtime with months going by before you even find out that there is a bug somewhere in the code.

Mr Blue

I'm sorry Mr Red but you are completely and utterly wrong: this has absolutely nothing to do with types or IDEs and absolutely not "better" than some string. You are totally missing the point here. A Boolean value has absolutely no semantics beyond a binary flag. It doesn't describe anything. That's why it is a code smell. foo(42, true, false) vs foo(42, true, true) vs foo(42, false, false). That's a useless API that tells you nothing about what the calls mean. You have to know the implementation: so Boolean flags are a leaky abstraction.

Mr Red

You are certainly entitled to your strong opinions, Mr Blue, but that does not mean that I am wrong. of course that the words true and false do not have a semantic value. The semantic value is in the argument name.

There is one thing that is much worse than the lack of semantics and that's the passing of strings which are prone to errors and are less efficient in checking.

IMO a function that is declared as:
find(string input, string substring, boolean ignoreCase)
which you would then call as
find("abcde", "Bc", true)
is much better than
find(string input, string substring, string flags)
which will be called as
find("abcde", "Bc", "ignoreCase")
or maybe it was "notCaseSensitive"? or perhaps "caseInsensitive"? or...

Mr Purple

That article "avoid boolean parameters" is
from 2004, at that time I was still using Allaires Kawa Editor (I
think) and for this editor that argument maybe was valid, but in Eclipse or
Netbeans a simple mouse over tells me the function arguments, problems
today in Java is not having boolean arguments, problem is having 100
methods in a class that are slightly different and this is exactly what the
article suggest, to more methods! I always try to reduce methods in my
classes.

Maybe it is handy to read existing code when you have 1000 of slightly
different methods, but it is certainly bad for writing code.

That is the point, Mr Red's way is handy to write code, Mr Green's solution is handy
to read code. Mr Red knows that I hate to at new functions to The Application, I'm
afraid of the "PHP Effect", having 1000 of functions doing slightly the same.

Mr Green

Be it from 2004, is there industry consensus that has decided "clean code" is not still something to strive for? Or that the premises offered somehow are now incorrect? (Not a purposely rhetorical question, but I suspect know the answer)?

Considering the function at hand, a boolean argument - whether or not boolean arguments are poor programming or not - is simply painting one's self into a corner anyhow. It's not a case of "short: yes or no": In English - I cannot vouch for other languages - there are at least three accepted shortenings of a day of the week: "d", "dd", "ddd" (with "dddd" being "long-form"). How does one reflect that with a boolean? If we have one or the other, which are we picking?

Mr Purple has speciously posed that a boolean argument is better than multiple functions. This is tenuous, but irrelevant here: no-one's suggesting that. I suggested having a mask which one can use which offers all the required functionality (a boolean doesn't), has an already-set precedent in the language (date/time format), and also doesn't suffer from Mr Red's concern that invalid strings can cause unexpected errors. If the mask is not one of the supported patterns: the code can simply error.

Mr Red does offer a reasonable suggestion that Java offers enumerations to cover optionality rather than strings. CFML doesn't, and there's a healthy precedent there to invalidate that position, but if we were to decide that using enumerations are better than strings... cool... implement that.

But using a boolean is an invalid suggestion here.

Mr Red

I'm sorry, I should have clarified: my comments were about the general statement against boolean arguments, and not specifically to the Format functions.

Formatting functions and their like should take a mask for an argument -- I wasn't suggesting that we add 17 boolean arguments instead of a mask ;)

So what do you think here? Obviously the conversation (apparently) strayed from the original issue a bit, but I think the positions everyone states are relevant to the function at hand, as well as to the notion of how to write functions in general.

What do you think?

--
Adam