The nice thing working for Trolltech was (among others) learning the principles behind the good API. The article Designing Qt-Style C++ API from Matthias 6 years ago is still a good reading till today. The content itself is now expanded into the wiki page API Design Principles.
The major premise behind a good API is rather straightforward:
the code is usually written once but read many times.
When writing the code, the developer has the time she needs to look at the API documentation and digest it. When someone else reads the code (for review or bug fix), she may not always have the API documentation handy. While this is just common sense, wait until you finish reading and see why this important fact is still overlooked these days.
Note: Qt is in C++, but surprisingly the same API design principles apply to the (wonderful) world of JavaScript, which is the focus in this blog post.
George Boole, inventor of the Boolean logic.
For this particular discussion, I’ll pick my favorite API design mistake: boolean trap. On this topic, the above API Design Principle wiki page says that
it’s almost invariably a mistake to add a bool parameter to an existing function
Let’s start with the textbook example: guess what this code means?
widget.repaint(false);
Without looking at the documentation, the usual suspect is don’t repaint the widget. Then, you look at the documentation and it refers the function argument as immediate, which is true if you want immediate painting or false for deferred painting. Thus, the correct behavior implied by the above code is actually repaint this widget later, which is miles away from your initial guess.
One possible solution to this problem is to use explicit function argument. In the C++ world, this can be solved using enum, e.g. widget.repaint(WidgetClass::Deferred). In the JavaScript world, the alternative is using an object literal such as,
widget.repaint({ immediate: false });
or the more verbose variant:
widget.repaint({ mode: "immediate" });
or just create a different function for that purpose:
widget.repaintLater();
There will be a concern of performance since an object is more expensive than just a simple boolean literal. Thus, profile your code carefully and set a sensible compromise if this above line is in your hot path. On the other hand, I also do believe that modern future JavaScript engines would be smart enough to optimize such usages that the speed penalty is negligible.
Another classic is the confusion during construction. Your user interface needs a bunch of sliders to allow the user to choose some values. Here is one line in the code for you to review:
var opacitySlider = new Slider(true);
Mysteriously, there are also lines similar to:
var volumeSlider = new Slider(false);
It turns out that true there means a horizontal slider and false means a vertical slider. Of course, the easiest way to clear this confusion is to actually name the object HorizontalSlider and VerticalSlider and get rid of the boolean argument. Heck, who knows someday you’ll need a diagonal slider!
You may scream, "Of course, I won’t be too idiot to make those rookie mistakes!". Well, in the following paragraphs I’ll give examples of actual boolean traps in the API of several well-known JavaScript libraries and frameworks out there (I try to be unbiased). Consider that there are millions of developers using the libraries in real-world web applications, imagine the exposure of the traps.
For each of this case, imagine you are doing a code review. Your amazing coworker wants to commit a patch and he consults you to check your opinion.
to be or not to be
This is the same as the textbook example, but coming from a real framework:
stackView.updateHeight(false);
Yes, that false again refers to immediate or not. To the untrained developer, the above line feels like don’t update the height. A real crazy one might even stretch it to update the width!
Here is another one. To facilitate easy iteration of child widgets, you can use next() function which would get you the next sibling. But then, the code looks like:
widget.next(true);
which actually does the extra magic (because of the true value) that the very first child widget will be returned if you hit the last one. In other words, true there stands for circular. An innocent value which does too much behind your back. Well, good luck trying to review that kind of code.
Another dangerous venture:
widget.destroy(false);
which potentially leads you to think don’t destroy this widget. You can’t be more wrong, the function actually still destroys your widget, but it leaves the DOM associated with the widget intact. Only if the argument is true then actually every related DOM pieces is also tore torn down.
optionally undecipherable
Now that we have the slider for the UI, we need to preset the value:
volumeSlider.setValue(90, false);
Another boolean horror! The documentation reveals that false there indicates that the slider should not animate the movement of its indicator from the old value to the new value. By default, it will show the animation but since we want to set the initial value, the animation will be distracting and needs to be off. How about writing it like this instead?
volumeSlider.setValue(90, { animation: false } );
There is this list view of all your customers. We want to find out who live in a certain city. Can you guess what the last optional argument refers to?
customerView.filter('address', 'sunnyvale', false);
Oh, apparently the API documentation refers it to caseSensitive! Just by looking at it, this is not obvious and it could mean an entirely different thing, anything from exactMatch to highlightMatchedLine. One possible workaround:
customerView.filter('address', 'sunnyvale', { caseSensitive: false });
the more, the merrier
While one boolean argument is already confusing, two boolean arguments can’t be more fun.
To handle layout, often there is a line of code that looks like:
cmp.setCentered(true, false);
Again, a trip to the API doc enlightens the reviewer that the function signature is actually setCentered(centered, autoUpdate). This is confusing as setCentered(centered) only is probably fine, it’s just like a property setter, but the interplay of the autoUpdate argument forces the brain to think harder.
Note that a pair of values like that, especially in the context of centering/geometry purpose, might provoke a different interpretation: center vertically and horizontally. This is arguably the most sensible one which comes to mind if one sees that code.
Here is another one:
menu.stop(true, false);
The boolean values there refer to clear the animation queue or not and go to the animation or not, respectively. They are not even remotely related. What is your best educated guess if you did not know this beforehand?
Of course, why stop at two if you can have more?
event.initKeyEvent("keypress", true, true, null, null, false, false, false, false, 9, 0);
double negative
Now, coming back to property setter, this is one valid use of boolean argument, e.g. dialogBox.setVisible(true). However, care must be taken so that there is no such double negative. Especially for non-native speakers, double negative requires an extra careful measure to make sure that the right meaning is communicated.
If I wake you at midnight and ask you this question "if invisible is false, does that mean my component is shown or hidden?", there is a chance you answer it incorrectly.
Real-world examples of double negative follow:
volumeSlider.setThumbsDisabled(false); component.setDisabled(false); filter.setCaseInsensitive(false);
Would you be less confused if this is what you read instead?
volumeSlider.setThumbsEnabled(true); component.setEnabled(true); filter.setCaseSensitive(true);
The same principle applies to active vs inactive, modified vs unmodified, defined vs undefined, selected vs unselected, etc.
By now, hopefully you got the idea of various risky uses of boolean argument. Feel free to share your favorite freak-out moment as you encounter such a similar trap.
Most importantly, next time you design an API function, remember George Boole and don’t let him down!
Update: Some people on Reddit pointed out that they would not interpret widget.repaint(false) as do not repaint. First of all, it’s subjective. In some languages it can be understood as repaint not, which is effectively a negation. Also, the context might pollute, e.g. if there is fooWidget.show(false) (which means do not show) right before, then it may influence a similar conclusion for the repaint issue. I was also not clear that any crazy possible interpretations are just examples, substitute them with your own imaginations. The fact that everyone can propose a different interpretation is the premise: ambiguity begets insanity.
[source]
