Friday 21 September 2012

One Definition to Rule them all

Generally I am a little bit suspicious of people who can quote the C++ standard, knowing the exact definitions and all the obscure intricacies should only be necessary for compiler writers. The rest of us should get by with a broad understanding of how things work and the compiler should catch us when we stray.

In fact, and as an aside, I would actually consider too much knowledge of operator precedence to be positively harmful. People who know things can't help themselves but to use that knowledge and end up leaving out unnecessary braces in compound statements because it's obvious to them that they are unnecessary. Some idiot who hasn't memorised the rules (like me) then inherits the code, makes an incorrect assumption about what it's trying to do, and screws it up.

Bytes are cheap, just use brackets.

At least that was what I thought until I got bitten firmly in the arse by this little beauty:
http://stackoverflow.com/questions/6379422/c-multiple-classes-with-same-name
Which if I had been more familiar with standard I would have spotted immediately.

In my case I had a little interface I needed to mock for a couple of tests.

class ThingResponder
{
public: 
   virtual void respond(int) = 0;
};

In the first test I didn't care about it.

test1.cpp:

class MyThingResponder : public ThingResponder
{
public: 
   virtual void respond(int) {}
};

void test1::someTest()
{
    MyThingResponder responder;
    foo(responder);
    ...

But in the second test I wanted to verify it was called, so did something a little different

test2.cpp:

class MyThingResponder : public ThingResponder{
public: 
    std::vector<int> responces;
    virtual void respond(int responce)
    {
        responces.push.back(responce);
    }
};

void test2::someTest()
{
    MyThingResponder responder;
    foo(responder);
    ...

Can you guess what happened next.

I had already broken the one definition rule, this did not result in any build errors or warnings, instead when the calling frame wanted to allocate an object of MyThingResponder it used the local definition, but when it called the constructor it used the one from test2.cpp. This meant after I wrote test2, test1 started crashing in weird ways because the constructor for MyThingResponder was initialising a std::vector that wasn't there and screwing up a load of other local variables.

Had it been the other way around the responces vector would have never been initialised and it would have been even harder to track down the error.

These are the things that scared me.

  1. As we move to using more functors and the like, small locally defined classes with generic names will become more common.
  2. There is no way to guard against this. Anyone else's new class could break your code.
And these are the things I realised:
  1. I am going to put everything in a namespace from now on. Everything!
  2. I suppose I do kind-of have to know the obscure intricacies of the C++ standard after all.

No comments:

Post a Comment