czwartek, 28 lutego 2013

Recommended reading: Clean Code

A week ago I started reading Uncle Bob's book "Clean Code". Although I'm still not yet half way through the book, I want to recommend this book to anyone who has not read it yet. One of the many things that impressed me during the reading was this:

In in the book, Robert Martin makes several times the point that the bad code, such as one with long functions, many indentation levels, cryptic names and complicated conditional expressions inside those functions is simply hard to read. This may be an astonishing this to hear from a professional, at a first glance. Indeed, through our education and early professional work we become used to the notion that "real professional" is able to decypher really complicated code without a hitch. We are also often impressed by our colleagues who through years of dedicated work have become fluent in understanding of extremely bad written and uncomprehensible modules.

Martin tells us that this is not the right way to look at things. Although it is not said strictly in the book, it almost reads like: hey, look, even I, with all my years of experience and proficiency in computer languages, am not feeling comfortable when I look at a bad-written module and it takes me a lot of time to even vaguely decypher what it does and how it does it.

We should not base our professional value system on how comlicated and badly-written code one is able to comprehend.

środa, 27 lutego 2013

Code Kata: Recently opened files

Kata #2: Recently opened files
 
Implement a container that has the well known functionality of "Recently opened files" that you know from various applications you use:
  • the container must be able to list file paths that it stores
  • if file A is opened and right after that we ask the container to list the file paths it stores, the path to file A will be listed as the first one
  • the number of stored file paths is limited; when the limit is exceeded the container forgets least recently used file path
  • in the list of recently open files, each file path can appear once and only once

wtorek, 26 lutego 2013

Code Kata: Vending Machine

Kata #1: Vending Machine

Your task is to implement a piece of software for a vending machine that sells sweets. The machine accepts coins and must be capable of returning change. It is up to you to decide on how inserted coins and change that is given out are represented. Consider the following variations of this exercise:
  • Simplification: you have an unlimited number of coins of every kind
  • Realistic: the machine is loaded with coins at the beginning and you have a limit on the number of coins of every kind
  • More realistic: the machine adds the inserted coins to respective coin slots and uses them to serve further transactions
  • Even more realistic: the machine tries to optimize the way of giving out coins - if there is danger of giving out too many coins of kind A, and there are is surplus of coins of kind B, then more of B coins are used to produce change
  • Futuristic: the machine asks you to provide it with coins that would help it return the proper change
How would your implementation of vending machine react if the amount of money inserted were less than the price of the chosen chocolate snack?

poniedziałek, 25 lutego 2013

Piotr's Less Obvious Advice on Google Mock: State maintenance

Google Mock provides several ways to maintain state inside mock objects. One way of implementing state maintenance is with SaveArg. Consider the following example.

We have a class Configurator, which allows a caller to set and get values of a parameter:

class Configurator
{
    public:

    virtual ~Configurator() {}

    virtual void setParamX(int n) = 0;
    virtual int getParamX() = 0;
};


And we have a class Client that calls Configurator's methods and it also has a method incParamXBy, that can be used to increase the current value of paramX by a certain value.

class Client
{
    public:

    Client(Configurator & cfg);
    virtual ~Client() {}

    void setParamX(int n);
    void incParamXBy(int n);
    int getParamX();

    private:

    Configurator & _cfg;
};

incParamXBy internally calls setParamX and getParamX on Configurator:

void Client::incParamXBy(int n)
{
    _cfg.setParamX(_cfg.getParamX() + n);
}

Let's assume that the initial value of paramX is A and that we want to increase paramX by B each time we call incParamXBy. Our expectation is that if incParamXBy is called for the first time, it will result in calling cfg.setParamX(A+B).
  • second call of incParamXBy(B) will result in calling cfg.setPAramX(A + 2*B)
  • third call: cfg.setPAramX(A + 3*B), and so on...

Interaction between Client and Configurator in this case is such that Client relies on Configurator (issues getParamX()) in order to correctly calculate the parameter for next setParamX() call. If we want to test Client object, we need a MockConfigurator that is able to remember its current value of the parameter we are increasing:

class MockConfigurator : public Configurator
{
    public:

    int paramX;
    int * paramX_ptr;

    MockConfigurator()
    {
        paramX = 0;
        paramX_ptr = &paramX;
    }
   
    MOCK_METHOD1(setParamX, void(int n));
    MOCK_METHOD0(getParamX, int());
};

MockConfigurator has a public member paramX. We will use paramX to store current value of the parameter. We could have put paramX outside of MockConfigurator, in the test code, but I think it makes more sense to bind the parameter with the object.

And now the test:

MockConfigurator cfg;
Client client(cfg);

int inc_value = 10;

//getParamX will be called a number of times.
//If it is called, we will return the value pointed to by paramX_ptr.
//Returning with ReturnPointee is necessary, since we need to have
//the actual (updated) value each time the method is called.

EXPECT_CALL(cfg, getParamX())
    .Times(AnyNumber())
    .WillRepeatedly(ReturnPointee(cfg.paramX_ptr));

//SaveArg stores the 0th parameter of the call in the value pointed to by paramX_ptr (paramX)
//expectation 3
EXPECT_CALL(cfg, setParamX(cfg.paramX + 3*inc_value))
    .Times(1)
    .WillOnce(DoAll(SaveArg<0>(cfg.paramX_ptr), Return()));
//expectation 2
EXPECT_CALL(cfg, setParamX(cfg.paramX + 2*inc_value))
    .Times(1)
    .WillOnce(DoAll(SaveArg<0>(cfg.paramX_ptr), Return()));
//expectation 1
EXPECT_CALL(cfg, setParamX(cfg.paramX + inc_value))
    .Times(1)
    .WillOnce(DoAll(SaveArg<0>(cfg.paramX_ptr), Return()));

client.incParamXBy(inc_value); //this will match expectation 1
client.incParamXBy(inc_value);
//this will match expectation 2
client.incParamXBy(inc_value); //this will match expectation 3

Is it the only way to maintain state inside a mock object? Of course not. First of all, we could have an equivalent test without maintaining state inside out mock object at all. We could have written all method calls and all expectations using pre-calculated values that we know validate Client's behaviour. The point is that state maintenance gives us more flexibility that may be advantageous in case we want to write a number of similar tests (e.g. test the same behaviour for a range of parameter values or test different behaviours that rely accessing value previously stored in a mock).

Secondly, we can always implement state maintenance by using Invoke. Calling another function or method allows us not only to an equivalent to SaveArg, but also more complex state maintenance cases.

Piotr's Less Obvious Advice on Google Mock: Returning new objects from a mock

Google Mock provides a way to return newly created objects from a mock method. Suppose we have a 
Generator class that is supposed to generate new objects when createNewRecord method is called:

class Generator
{
    public:
    virtual ~Generator() {}
    virtual Record * createNewRecord() = 0;
};

...and suppose we want to mock this class:

class MockGenerator : public Generator
{
    public:
    MOCK_METHOD0(createNewRecord, Record * ());
};

Suppose the caller class Client has run method defined as follows:

void Client::run()
{
    for(int i = 0; i < 3; i++)
    {
        rec_tab[i] = gen.createNewRecord();
    }
}

We want the mock to return a pointer to a new object each time createNewRecord is called. This is how we can express this in the test code:

TEST(ClientTest, CanRun)
{
    MockGenerator gen;
    Client c(gen);

    EXPECT_CALL(gen, createNewRecord())
        .Times(3)
                 //this is equivalent of returning new Record(1,2,3)
        .WillOnce(ReturnNew<Record>(1,2,3))
        .WillOnce(ReturnNew<Record>(2,3,4))
        .WillOnce(ReturnNew<Record>(3,4,5));

    c.run();
}


Creating and passing new objects through a method parameter
 
But what if we want to return a new object through a method parameter? Sometimes caller passes a pointer-to-pointer to a method that is supposed to create a new object and assign its address to the method parameter. Let's modify our example above so that:
  • the caller (class Client) has a private member:
Record * rec;
  • the caller (class Client) has the following method:
void Client::getNewRec()
{
        // we are passing Record ** (pointer to pointer) to createNewRecord
        // and Generator is supposed to allocate memory
    gen.createNewRecord(&rec);
}
  • our Generator.h has the following declaration:
virtual void createNewRecord(Record ** rec) = 0;
  • which results in the following declaration in MockGenerator.h:
MOCK_METHOD1(createNewRecord, void (Record ** rec));

How can we now emulate the desired behavior in a test code? A straightforward way of doing this would be:

TEST(ClientTest, GetsNewRec)
{
    MockGenerator gen;
    Client c(gen);

    EXPECT_CALL(gen, createNewRecord(_))
        .Times(1)
        .WillOnce(Invoke(my_create));

    c.getNewRec();
}

Where my_create is a small function that does the trick:

void my_create(Record ** rec)
{
         *rec = new Record(1,2,3);
}
But isn't it awkward? The need to define this small function, the hard-coded constructor's parameters... It would  be much more convenient if we had a Google Mock action that could actually create the new object and assign its address to the pointer. Then, in our test, we could simply write:

        .WillOnce(CreateAndPass<Record>(1,2,3));

The CreateAndPass action must be smart enough to create an object of a certain type (Record), create it with the parameters that we specify (1,2,3) and do the proper assignment to the pointer. For a constructor that takes three parameters, as in the example above, our new action would look like this:

ACTION_TEMPLATE(CreateAndPass,
                HAS_1_TEMPLATE_PARAMS(typename, T), // the type of object to be created
                AND_3_VALUE_PARAMS(p0, p1, p2))         // constructor parameters
{
    // we are using TR1; assign the pointer of the newly created object to dereferenced call parameter
    // this is the only parameter of the call in this case, so it has the index of 0

  *(::std::tr1::get<0>(args)) = new T(p0, p1, p2);
}

That's it. This example can be easily extended for different constructors that take less or more parameters. Since the action is templatized, we can use it to construct objects of any class.

Piotr's Less Obvious Advice on Google Mock: Mutual method calls

Imagine that object A calls method M of object B and B is a mock object. Let's assume further that if B were real, calling the M method would result in B calling a method on A (M would call an A's method from its body). Suppose we want to simulate the same behavior with a mock object. We have a "master" and "slave" objects called Starter and Startee, respectively.

class Starter
{
 public:
  Starter(Startee & startee, int vol);
  virtual ~Starter() {}

  void start();
  void configure();

 private:
  Startee & startee_;
  int vol_;

  Starter();
};

// implementation of the above Starter methods
Starter::Starter(Startee & startee, int vol) : startee_(startee), vol_(vol)
{
}

// Starter will call run method on startee
void Starter::start()
{
  startee_.run();
}

// this method is supposed to be called by Startee as a result of calling startee_.run()
void Starter::configure()
{
  startee_.setVolume(vol_);
}

And now, Startee:

class Startee
{
 public:

  virtual ~Startee() {}

  virtual void run() = 0;
  virtual void setVolume(int volume) = 0;
};

And MockStartee:

class MockStartee : public Startee
{
 public:

  MOCK_METHOD0(run, void());
  MOCK_METHOD1(setVolume, void(int volume));
};

In the test, we will need to tell the mock object to call configure method once the Starter object calls run method on MockStartee:
TEST(StarterTest, MutualCall)
{
  MockStartee startee;
  Starter starter(startee, 10);
 
  EXPECT_CALL(startee, setVolume(10))
    .Times(1)
    .WillOnce(Return());
 
  EXPECT_CALL(startee, run())
    .Times(1)
    .WillOnce(WithoutArgs(Invoke(&starter, &Starter::configure)));

  starter.start();
}

Piotr's Less Obvious Advice on Google Mock: Mocking destructors

Can we verify that a mock object is properly destroyed? Of course! There is a couple of subtle differences between mocking regular functions and mocking destructors. Let's suppose we have a Grinder object that destroys any Piece object passed to its grind method:

void Grinder::grind(Piece * piece) {
  delete piece;
}

Furthermore, Grinder can accumulate a list of Pieces (actually, let it be a list of pointers to Pieces) for destruction that will take place when Grinder itself is destroyed. To add a Piece to the list, we define:

int Grinder::enqueue_piece(Piece * piece) {
  list_of_pieces_.push_back(piece);
  return list_of_pieces_.size();
}

Now, to keep the promise of destroying the queued pieces, Grinder's destructor is defined as follows:

Grinder::~Grinder() {
  for(list<Piece*>::iterator it = list_of_pieces_.begin(); it != list_of_pieces_.end(); it++) {
    delete *it;
  }

}

But how can we mock the destructor of Piece so that we can verify that Grinder really destroys Pieces in both scenarios (on grind method call and on Grinder destruction)? Well, we can't really mock Piece's destructor itself, but we can use a workaround: it's enough to define MockPiece destructor so that it calls another function, such as destroy, that will be used as a signal for us that the destructor has been called.

class Piece {
 public:
  virtual ~Piece() {}
};

class MockPiece : public Piece {
 public:
  MOCK_METHOD0(destroy, void());
  virtual ~MockPiece() { destroy(); }
};

Now, a test like:

TEST(Grinder, CanGrindPiece) {
  MockPiece * piece = new MockPiece;
  Grinder grinder;

  EXPECT_CALL(*piece, destroy());

  grinder.grind(piece);
}

Will pass correctly (we know that grind method deletes the object passed as a pointer argument). But what about this test:

TEST(Grinder, CanGrindWhenDies) {

  MockPiece * p1 = new MockPiece;
  MockPiece * p2 = new MockPiece;
  MockPiece * p3 = new MockPiece;
  list<Piece*> list_of_pieces;
  list_of_pieces.push_back(p1);
  list_of_pieces.push_back(p2);
  list_of_pieces.push_back(p3);

  Grinder grinder;

  EXPECT_CALL(*p1, destroy());
  EXPECT_CALL(*p2, destroy());
  EXPECT_CALL(*p3, destroy());

  grinder.enqueue_piece(p1);
  grinder.enqueue_piece(p2);
  grinder.enqueue_piece(p3);

  //Grinder dies after this line
}

If you try out test example above with the correct implementation of Grinder's destructor, it will pass. But try to comment out the delete statement in Grinder's destructor and see what happens. Google Mock prints error messages like:

.//Grinder_test.cpp:34: ERROR: this mock object (used in test Grinder.CanGrindWhenDies) should be deleted but never is. Its address is @0x809d5e0.

but it still reports that the test itself passed! We would like to have Google Mock report test failure in this case, wouldn't we? The trouble is that Grinder dies when our test is already finished (when it goes out of TEST scope - it is an automatic object). The workaround here is to add another "helper" test that will verify our expectations after  the proper test has been finished. Let's do the following modification: move MockPieces p1, p2 and p3 from CanGrindWhenDies test to global scope. Then, let's add a short helper test:

TEST {
::testing::Mock::VerifyAndClearExpectations(p1);
::testing::Mock::VerifyAndClearExpectations(p1);
::testing::Mock::VerifyAndClearExpectations(p1);
}

That's it: this implementation of expectations and verification will yield correct results ("helper" test passing or failing) depending on whether Grinder's destructor is implemented correctly.

Exercise: try to move VerifyAndClearExpectations statements to CanGrindWhenDies test and see what happens when you run the test with correct and incorrect implementation of Grinder's destructor.

Piotr's Less Obvious Advice on Google Mock: Assigning values through method calls

On many occasions, we may want to test a real object by passing to it prepared buffers of data (characters, bytes, etc.) that are filled in when the object calls a mock method. This can be achieved by using SetArrayArg.

Suppose we have a Client object that implements a find method, which takes an array of characters as input argument. The method queries a Driver object for chunks of data (other arrays of characters of the same size as the input argument) and compares each chunk with the input array:

void Client::find(char * buf)
{
    char tmp_buf[buflen];
    bool found = false;

    while(!found && _drv.getDataChunk(tmp_buf))
    {
        int i = 0;
        while(i < buflen && buf[i] == tmp_buf[i])
        {
            i++;
        }

        if(i == buflen)
            found = true;
    }

    if(found)
        cout << "Found matching chunk of data." << endl;
    else
        cout << "No matching chunk of data found." << endl;
}

Client assumes that Diver implements the following contract:
  • if there is no more data chunks, getDataChunk will return false,
  • otherwise, it will return true and will fill in the buffer passed to it through pointer as input, with next chunk of data.
class Driver
{
    public:

    virtual ~Driver() {}

    virtual bool getDataChunk(char *) = 0;
};

class MockDriver : public Driver
{
    public:

    MOCK_METHOD1(getDataChunk, bool(char *));
};

Let's now consider a test in which we want to simulate that a matching chunk of data is found on the first call of getDataChunk.

TEST(ClientTest, FindsAMatchOnFirstCall)
{
    MockDriver drv;
    Client client(drv);

        //this is the buffer that will be searched for
    char buf [] = {'a','a','a','a','a','a','a','a'};

        //this is the matching buffer that will be returned by MockDriver
    char bufA [] = {'a','a','a','a','a','a','a','a'};

    EXPECT_CALL(drv, getDataChunk(_))
        .Times(1) //we are expecting just one call, since the first call will return the matching buffer

                //the mock will return true and before doing so it will copy the contents of bufA into the table
                //pointed to by the pointer passed to getDataChunk method as the input parameter
        .WillOnce(DoAll(SetArrayArgument<0>(bufA, bufA+8), Return(true)));
   
    client.find(buf);
}


Exercise: try to modify the code above to validate your expectations on Client::find method when there is no matching chunk of data.