by Ursego » 21 Feb 2013, 22:04
If a function returns Boolean then its name should convey the meaning of the returned value to make the business logic in the calling scripts easily understandable. Calling that function as a condition of an if statement must produce a correct real-English sentence.
Here are some examples of well-named Boolean methods: rowIsValidated, ordersAreOk, fileNameExists. A strange tradition dictates to put the words "Is"/"Are"/"Does"/"Do" in the beginning (like isRowValidated, areOrdersOk, doesFileNameExist etc.), but I personally don't like that approach because it produces incorrect English in the calling scripts: we say "if row is validated then do something", not "if is row validated...". Of course, it's not an issue of high importance - you can even omit "Is"/"Are" at all, and the names still will be fine: rowValidated, ordersOk.
This rule has an exception - getters of Boolean fields (instance variables): they must follow the convention getField or isField. In Java, that is one of the requirements in definition of JavaBean, and that makes sense not only for programs (which can automatically extract the field name from the getter name), but for programmers too: for example, the name isInvoicePrinted conveys the idea that a Boolean field invoicePrinted exists in the class.
Never use verbs in imperative to name Boolean methods
For example, the following names are absolutely unacceptable for methods returning Boolean: validateRow, checkOrders, checkFileExistence - we don't say "if validate row then save", "if check orders then print", "if check file existence then open file"!
From the book "Clean Code":
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");
...
}