Command Query SeparationFunctions should either do something or answer something, but not both. Either your function should change the state of an object, or it should return some information about that object. Doing both often leads to confusion. Consider, for example, the following function:
- Code: Select all
public boolean set(String attribute, String value);
This function sets the value of a named attribute and returns true if it is successful and false if no such attribute exists. This leads to odd statements like this:
- Code: Select all
if (set("username", "unclebob"))...
Imagine this from the point of view of the reader. What does it mean? Is it asking whether the "username" attribute was previously set to "unclebob"? Or is it asking whether the "username" attribute was successfully set to "unclebob"? It's hard to infer the meaning from the call because it's not clear whether the word "set" is a verb or an adjective.
The author intended set to be a verb, but in the context of the if statement it feels like an adjective. So the statement reads as "If the username attribute was previously set to unclebob" and not "set the username attribute to unclebob and if that worked then...." We could try to resolve this by renaming the set function to setAndCheckIfExists, but that doesn't much help the readability of the if statement. The real solution is to separate the command from the query so that the ambiguity cannot occur.
- Code: Select all
if (attributeExists("username")) {
setAttribute("username", "unclebob");
...
}