Should one always know what an API is doing just by looking at the code?

Question

Recently I have been developing my own API and with that invested interest in API design I have been keenly interested how I can improve my API design.

One aspect that has come up a couple times is (not by users of my API but in my observing discussion about the topic): one should know just by looking at the code calling the API what it is doing.

For example see this discussion on GitHub for the discourse repo, it goes something like:

foo.update_pinned(true, true);

Just by looking at the code (without knowing the parameter names, documentation etc.) one cannot guess what it is going to do - what does the 2nd argument mean? The suggested improvement is to have something like:

foo.pin()
foo.unpin()
foo.pin_globally()

And that clears things up (the 2nd arg was whether to pin foo globally, I am guessing), and I agree in this case the later would certainly be an improvement.

However I believe there can be instances where methods to set different but logically related state would be better exposed as one method call rather than separate ones, even though you would not know what it is doing just by looking at the code. (So you would have to resort to looking at the parameter names and documentation to find out - which personally I would always do no matter what if I am unfamiliar with an API).

For example I expose one method SetVisibility(bool, string, bool) on a FalconPeer and I acknowledge just looking at the line:

falconPeer.SetVisibility(true, "aerw3", true);

You would have no idea what it is doing. It is setting 3 different values that control the "visibility" of the falconPeer in the logical sense: accept join requests, only with password and reply to discovery requests. Splitting this out into 3 method calls could lead to a user of the API to set one aspect of "visibility" forgetting to set others that I force them to think about by only exposing the one method to set all aspects of "visibility". Furthermore when the user wants to change one aspect they almost always will want to change another aspect and can now do so in one call.

Answer 1

Your desire not to split it out into three method calls is completely understandable, but you do have other options besides boolean parameters.

You could use enums:

falconPeer.SetVisibility(JoinRequestOptions.Accept, "aerw3", DiscoveryRequestOptions.Reply);

Or even a flags enum (if your language supports it):

falconPeer.SetVisibility(VisibilityOptions.AcceptJoinRequests | VisibilityOptions.ReplyToDiscoveryRequests, "aerw3");

Or you could use a Parameter Object:

var options = new VisibilityOptions();
options.AcceptJoinRequests = true;
options.ReplyToDiscoveryRequest = true;
options.Password="aerw3";
falconPeer.SetVisibility(options);

The Parameter Object pattern gives you a few other advantages that you may find helpful. It makes it easy to pass around and serialize a set of parameters, and you can easily give unspecified parameters "default" values.

Or you could just use boolean parameters. Microsoft seems to do it all the time with the .NET Framework API, so you could just shrug and say "if it's good enough for them, it's good enough for me".

Answer 2

Obviously, there are always exceptions to the rule, but as you explained well by yourself, there are certain advantages in having a readable API. Boolean arguments are particularly bothersome, as 1) they do not reveal an intent and 2) they imply that you call a function, where you actually should have two, because different things are going to happen depending on the boolean flag.

The main question is rather: how much effort do you want to invest into making your API more readable? The more outwards-facing it is, the more effort can easily be justified. If it is an API that is used only by one other unit, then it's not that big of a deal. If you're talking about a REST API where you plan to let the whole world lose upon it, then you may as well invest some more effort into making it more understandable.

As for your example, there is a simple solution: Apparently, in your case visibility is not just a true or false thing. Instead, you have a whole set of things that you consider "visibility". One solution may be to introduce something like a Visibility class, which covers all these different kinds of visibility. If you further apply the Builder-pattern for creating these, you may end up with code like the following:

Visibility visibility = Visibility.builder()
  .acceptJoinRequests()
  .withPassword("aerw3")
  .replyToDiscoveryRequests()
  .build();
falconPeer.setVisibility(visibility);