TDD Advanced Concepts : Command Query Separation Principle

Objectives


  • How to fix violation of Command Query Separation principle?
  • How to fix abuse of mocks?
  • How to write focused tests?
  • How to deal with external dependencies in your domain code?

Side Effect


A function or expression modifies some state or has an observable interaction with calling functions or the outside world in addition to returning a value. For example, a function might modify a global or static variable, modify one of its arguments, raise an exception, write data to a display or file, read data, or call other side-effecting functions. In the presence of side effects, a program's behavior depends on history; that is, the order of evaluation matters. Understanding a program with side effects requires knowledge about the context and its possible histories; and is therefore hard to read, understand and debug.

Side effects are the most common way to enable a program to interact with the outside world (people, file systems, other computers on networks). But the degree to which side effects are used depends on the programming paradigm. Imperative programming is known for its frequent utilization of side effects. In functional programming, side effects are rarely used.

Source: Wikipedia

Rails Example


Before

Here is an example of a badly designed API that violates command query separation principle:

user = User.new(params)

if user.save
  do something
else
  do something else
end

The save is inserting the record in the database. It is a command because it has a side effect. It is also returning true or false, so it is also a query.

After

user = User.new(params)
user.save

if user.persisted?
  do something
else
  do something else
end

Calculator Example


Before

Let's look at the calculator example that violates command query separation principle. Here is the calculator_spec.rb:

require_relative 'calculator'

describe Calculator, "Computes addition of given two numbers" do

  it "should add given two numbers that are not trivial" do
    calculator = Calculator.new
    result = calculator.add(1,2)

    expect(result).to eq(3)
  end
end

Here is the calculator.rb

class Calculator
  def add(x,y)
    x+y
  end
end

After

This version fixes the command query separation violation. Here is the calculator_spec.rb:

require_relative 'calculator'

describe Calculator, "Computes addition of given two numbers" do
  it "should add given two numbers that are not trivial" do
    calculator = Calculator.new
    calculator.add(1,2) 
    result = calculator.result

    expect(result).to eq(3)
  end
end

The add(x,y) method is a command. The calculator.result call is a query. Here is the calculator.rb

class Calculator
  attr_reader :result

  def add(x,y)
    @result = x + y
    nil
  end  
end

We have two choices : we can either return nil or the client can ignore the return value. If the API is for the public then returning nil explicitly will force the client to obey the CQS principle. If it is within a small team we can get away with ignoring the return value and making sure we obey the CQS principle.

Tweet Analyser Example


This is another example that violates Command Query Separation Principle. This is actually published by the Pragmatic Programmer's book on testing. If an author can write bad code like this then it is easy for other developers to write similar smelling code.

Before

Version 1 - tweet_analyser_spec.rb as found in the Pragmatic Programmer's book on testing:

class TweetAnalyzer
  def initialize(user)
    @user = user
  end
  def word_frequency
    {"one" => 1}
  end  
end

describe TweetAnalyzer do
  it "finds the frequency of words in a user's tweets" do
    user = double('user')
    analyzer = TweetAnalyzer.new(user)
    histogram = analyzer.word_frequency
    histogram["one"].should == 1
  end
end

It looks like client is tied to the implementation details (it is accessing a data structure) but it is actually any class that can respond to [] method. The command word_frequency is not only calculating the frequency but also returns a result.

After

Version 2 - tweet_analyser_spec.rb:

class TweetAnalyzer
  attr_reader :histogram

  def initialize(user)
    @user = user
  end  
  def word_frequency
    @histogram = {"one" => 1}
  end  
end

describe TweetAnalyzer do
  it "finds the frequency of words in a user's tweets" do
    user = double('user')
    analyzer = TweetAnalyzer.new(user)
    analyzer.word_frequency

    analyzer.histogram["one"].should == 1
  end
end

In this version, the command word_frequency() does not return a result. Is executes the logic to calculate word frequency. The histogram is now an exposed attribute that returns word frequency. So the command and query has been separated.

Version 3

Let's add a second spec that will force us to replace the fake implementation with a real one.

it "should return 2 as the frequency for the word two" do
  expected_tweets = ["one two", "two"]
  user = double('user')
  user.should_receive(:recent_tweets).and_return expected_tweets
  analyzer = TweetAnalyzer.new(user)

  analyzer.word_frequency  

  analyzer.histogram["two"].should == 2
end

This fails with the error:

  1) TweetAnalyzer asks the user for recent tweets
     Failure/Error: analyzer.histogram["two"].should == 2
       expected: 2
            got: nil (using ==)
     # ./tweet_analyzer_spec.rb:28:in `block (2 levels) in <top (required)>'

Finished in 0.00128 seconds
2 examples, 1 failure

Let's now implement the word_frequency for real. Change the word_frequency implementation like this:

class TweetAnalyzer
  # rest of the code same as before
  def word_frequency
    @histogram = Hash.new{0}
    @user.recent_tweets.each do |tweet|
      tweet.split(/\s/).each do |word|
        @histogram[word] += 1
      end
    end
  end  
end

Run the spec:

$rspec tweet_analyzer_spec.rb --color --format documentation

We get the failure message:

TweetAnalyzer
  finds the frequency of words in a user's tweets (FAILED - 1)
  should return 2 as the frequency for the word two

Failures:
  1) TweetAnalyzer finds the frequency of words in a user's tweets
     Failure/Error: @user.recent_tweets.each do |tweet|
       Double "user" received unexpected message :recent_tweets with (no args)
     # ./tweet_analyzer_spec.rb:10:in `word_frequency'
     # ./tweet_analyzer_spec.rb:22:in `block (2 levels) in <top (required)>'

Finished in 0.00132 seconds
2 examples, 1 failure

Failed examples:

rspec ./tweet_analyzer_spec.rb:19 # TweetAnalyzer finds the frequency of words in a user's tweets

We see that the second spec passed but now our first spec is broken. Let's fix this broken spec.

Version 4

Change the first spec in tweet_analyzer_spec.rb like this:

describe TweetAnalyzer do
  it "finds the frequency of words in a user's tweets" do
    expected_tweets = ["one two", "two"]
    user = double('user')
    user.stub(:recent_tweets).and_return expected_tweets
    analyzer = TweetAnalyzer.new(user)
    analyzer.word_frequency

    analyzer.histogram["one"].should == 1
   end
   # same code as before
end

Now both the specs pass. Note that we were able to make our tests pass without using a real user object. Our focus is only on testing the word frequency calculation not the user. User is a collaborator that the TweetAnalyzer interacts with to fulfill it's responsibility of frequency calculation.

We still have mocking going on in the second spec. Why should we care that recent_tweets method gets called on the user? We don't care about this in the second spec because our focus is not asserting on the outgoing message to the user collaborator object. This is an example of how mocks are abused. In this case mock is used instead of stub. Let's fix this in the second spec like this:

it "should return 2 as the frequency for the word two" do
  expected_tweets = ["one two", "two"]
  user = double('user')
  user.stub(:recent_tweets).and_return expected_tweets
  analyzer = TweetAnalyzer.new(user)

  analyzer.word_frequency  

  analyzer.histogram["two"].should == 2
end

This solution does not use mocking. It uses a user stub to enable the tests to run. This fixes abuse of mocks.

Version 5

Let's extract common setup to before() method.

tweetanalyzerspec.rb

class TweetAnalyzer
  attr_reader :histogram

  def initialize(user)
    @user = user
  end  
  def word_frequency
    @histogram = Hash.new{0}
    @user.recent_tweets.each do |tweet|
      tweet.split(/\s/).each do |word|
        @histogram[word] += 1
      end
    end
  end
end

describe TweetAnalyzer do
  before do
    expected_tweets = ["one two", "two"]
    @user = double('user')
    @user.stub(:recent_tweets).and_return expected_tweets    
  end  
  it "finds the frequency of words in a user's tweets" do
    analyzer = TweetAnalyzer.new(@user)

    analyzer.word_frequency

    analyzer.histogram["one"].should == 1
  end
  it "should return 2 as the frequency for the word two" do
    analyzer = TweetAnalyzer.new(@user)

    analyzer.word_frequency  

    analyzer.histogram["two"].should == 2
  end

end

We were green before and after refactoring.

Version 6

Focused spec test only one thing. If it is important that the user's recent tweets are used to calculate the frequency, write a separate test for that: tweet_analyzer_spec.rb:

describe TweetAnalyzer do
  ...  
  it "asks the user for recent tweets" do    
    expected_tweets = ["one two", "two"]
    user = double('user')
    user.should_receive(:recent_tweets).and_return expected_tweets

    analyzer = TweetAnalyzer.new(user)
    analyzer.word_frequency
  end  
end

In this case we are only interested in asserting on the message sent to the collaborating user object. We are not asserting on the state like the first two specs.

Version 7

Here is the refactored version of tweet_analyzer_spec.rb:

require_relative 'tweet_analyzer'

describe TweetAnalyzer do
  context 'Calculate word frequency' do
    before do
      expected_tweets = ["one two", "two"]
      @user = double('user')
      @user.stub(:recent_tweets).and_return expected_tweets    
    end

    it "finds the frequency of words in a user's tweets" do
      analyzer = TweetAnalyzer.new(@user)

      analyzer.word_frequency

      analyzer.histogram["one"].should == 1
    end

    it "should return 2 as the frequency for the word two" do
      analyzer = TweetAnalyzer.new(@user)

      analyzer.word_frequency  

      analyzer.histogram["two"].should == 2
    end    
  end

  context 'Collaboration with User' do
    it "asks the user for recent tweets" do    
      expected_tweets = ["one two", "two"]
      user = double('user')
      user.should_receive(:recent_tweets).and_return expected_tweets

      analyzer = TweetAnalyzer.new(user)
      analyzer.word_frequency
    end    
  end
end

tweet_analyzer.rb remains unchanged:

class TweetAnalyzer
  attr_reader :histogram

  def initialize(user)
    @user = user
  end  
  def word_frequency
    @histogram = Hash.new{0}
    @user.recent_tweets.each do |tweet|
      tweet.split(/\s/).each do |word|
        @histogram[word] += 1
      end
    end
  end
end

Again we are green before and after refactoring. So when do we stub and when do we mock? We can use the Command Query Separation Principle in conjunction with a simple guideline : Stub queries and mock commands. Ideal design will not stub and mock at the same time, since it will violate Command Query Separation Principle. See appendix of Essential TDD book for notes from Martin Fowler's article and jMock Home Page.


Related Articles


Ace the Technical Interview

  • Easily find the gaps in your knowledge
  • Get customized lessons based on where you are
  • Take consistent action everyday
  • Builtin accountability to keep you on track
  • You will solve bigger problems over time
  • Get the job of your dreams

Take the 30 Day Coding Skills Challenge

Gain confidence to attend the interview

No spam ever. Unsubscribe anytime.