DailyJS

DailyJS

The JavaScript blog.


Tagsoftware design
Featured

software design command query separation

Command-Query Separation in Practice

Posted on .

Let's end the week by learning about a software design principle that is easy to follow, yet a violation of it can cause bugs and coincidental correctness.

Command-query Separation (CQS) is the principle by which a method is defined to perform a query, execute a command, but never both. A query returns information about the state of the system, which can include anything from primitive data to objects, or even complex aggregation results. A command, on the other hand, changes the state of the system, usually by writing (persisting) modified objects or data. If we are working with a domain to manage Person objects a query method to find a person by name could be PersonQuery.find(name), and a command to persist an instance could be Person.apply().

Normal Violations

While CQS is a sensible principle, it is violated in everyday practice. This is not as troublesome as it sounds, depending on the type of violation. Strict adherence to CQS makes some methods impossible such as query methods that count the number of invocations, thus allowing a developer to query the number of queries.

/**
 * Finds a user by name and increments the
 * query counter.
 * @return the found Person or null if one was not found
 */
PersonQuery.prototype.find = function(name) {  
  this._queriesPerformed++;

  // get the Person from the datasource or null if not found
  var person = this.dataSource.find("Person", name);
  return person;
};

/**
 * @return the number of queries performed.
 */
PersonQuery.prototype.getQueriesPerformed = function() {  
  return this._queriesPerformed;
};

We could create an incrementQueriesPerformed() method instead of incrementing the counter within find(), but that would be excessive and burden the calling code.

// now the developer must synchronize the state of PersonQuery. Yikes!
var person = personQuery.find("someuser");  
personQuery.incrementQueryiesPerformed();  

In practice, CQS is rarely taken in its pedantic form because doing so would make certain types of batched/transactional calls cumbersome. Also, multi-threaded and asynchronous tasks would be difficult to coordinate.

Two classic violations are the Stack.pop() and HashMap.remove() methods as was the case in the LinkedHashMap article (inspired by Java's Map interface).

var map = new LinkedHashMap();  
map.put('myNum', 5);

// ...

var val = map.remove('myNum');  
alert(val); // prints "5"  

The remove() method is both a command and query because it removes the value associated with the key (command) and returns the value, if any, that was associated with the key (query). While the violation is subtle, the same behavior could have been implemented with a remove():void method.

var map = new LinkedHashMap();  
map.put('myNum', 5);

// ...

var val = map.get('myNum');  
map.remove('myNum');  
alert(val); // prints "5"  

As with many instances, the code to avoid CQS violation is clunky and overkill.

Although there are often violations of CQS at the method scale, many libraries violate CQS as an integral approach to their API design by combining both getter and setter behavior within the same method.

var person = new Person("someuser");

// one possibility
person.enabled(true); // set enabled to true  
var enabled = person.enabled(); // return true

// another possibility
person.value('enabled', true); // set enabled to true  
var enabled = person.value('enabled'); // return true  

I personally do not prefer this flavor of behavior overloading--instead recommending getEnabled() and setEnabled(Boolean)--but as long as the API is intuitive and documented then it's generally not a problem.

Dangerous Violations

There are, however, instances where CQS violations cause side effects that can be difficult to debug. This usually happens when code tries to be too clever or helpful by merging command and query behavior.

Consider the following code to query and create a Person:

var person = personQuery.find("someuser");  
if (person === null) {  
  person = new Person("someuser");
  person.apply(); // persisted
  console.log("Created the Person ["+person+"]");
} else {
  console.log("Person ["+person+"] already exists.");
}

The code attempts to find a Person and create one if it does not exist; further calls to find() will correctly return the Person. The logic is trivial and easy to understand, but imagine that a developer decides to reduce the effort by changing find() to create a Person if a result is not found.

PersonQuery.prototype.find = function(name) {  
  var person = this.dataSource.find("Person", name);

  if (person === null) {
    person = new Person(name);
    person.apply();
  }

  return person;
};

It can be argued that by combining both query and command behavior into find(), calling code can remove the if/else checks for null, thus making it simplier.

var person = personQuery.find("someuser");  
console.log("Person ["+person+"] exists.");  

The amount of code has been reduced, but we must consider other circumstances in which find() will be called.

As an example, when a user is creating a new Person via a standard HTML form, the system calls find() to ensure the name is not already taken. When find() was just a query method it could be called any number of times without affecting the system. But now it creates the Person it is attempting to find so a uniqueness constraint violation will occur.

// request one
var person = personQuery.find("newuser"); // returns newuser  
if (person !== null) {  
  alert("The name is already taken.");
}

// ... however, in a second request

// request two
var person = personQuery.find("newuser"); // Error (duplicate name)  
if (person !== null) {  
  // we can never reach this
  alert("The name is already taken.");
}

By adding command behavior to find(), the developer has accidentally broken the code. The original post-condition, the condition known to the code at the time, changed from returns null if no match was found to creates a Person and returns it if no match was found. Now the developer must spend time to refactor the original code, but maybe it's legacy or outside of his or her control. Care must be taken when changing method conditions.

One solution is to name the method more aptly to describe its behavior: PersonQuery.findOrCreate(). Then the method find() could be added as a pure query alternative, and calling code can choose which one to invoke under the circumstances.

Another solution is to not change find() at all. It worked before, and it will work in the future.

Conclusion

Strict adherence to CQS is nearly impossible to achieve in normal application programming, although some special applications might require such an architecture.

What should be kept in mind is that a looser definition of CQS is valid and useful in practice. A method should stick to being a command or query, but if both behaviors are needed the method name should reflect its dual nature and a non-query/command alternative should be offered.