2011-11-30

The Code-Off

The other day my team arranged a little code-off. A little programming contest. It was set up as a competition, where our team of four developers was divided into two sub-teams.

For the contest, I had found a class in our codebase functioning as a controller, that we would have to edit in relation to features we would be adding in the near future. We already had three similar features, added incrementally without really doing any cleanups afterwards, and we were going to add three more features that would behave in a the same or like manner. Here is a quick-look of the relevant parts of the class (totally obfuscated, of course.)
class Foo {
public:
  // detailes omitted
  Foo(/*contextual arguments*/);

  Bar * addKindA();
  Bar * addKindB();
  Bar * addKindC();

private:
  void addBar(Bar * b); // side-effect, called from the adders.
};
Following this pattern, the three next features would make the class look something like the following. Not exactly a maintainable class with low coupling:
class Foo {
public: // detailes omitted Foo(/*contextual arguments*/); Bar * addKindA(); Bar * addKindB(); Bar * addKindC(); Bar * addKindD(); Bar * addKindE(); Bar * addKindF(); private: void addBar(Bar * b); // side-effect, called from adders. };
We wanted to apply the open-closed principle on this class, so that we would not have to edit this particular class when adding the new features and instead provide extension-points for them. The mission for our code-off was thus: Within three hours, implement a solution that closes the class for edits w.r.t the planned features. Then we scheduled an hour afterwards, for discussion of the two resulting designs and choosing a winner. The winner then got the honor of being integrated in to the codebase.

One of the main goals with this "competition", was to give all on the team a practical experience with applying the open-closed principle in a real-life working scenario, as well as simplifying and lessen the chance of introducing errors to this class when adding the mentioned features later on.

The resulting designs was not to different - there are only in so many ways a class with three factory functions returning the same type can be closed from adding a new factory function. The "winning" design was able to isolate the Foo object from the details of object creation, conceptually looking like this:
class Factory {
virtual Bar * createKind(string kind) = 0; }; class Foo { public: // details omitted Foo(Factory * f, /* contextual arguments */); void addKind(string kindname){ Bar * b = f->createKind(kindname); addBar(b); } private: void addBar(Bar * b); };
With a superficial glance to the problem, the Factory argument to Foo's constructor might seem sub-optimal, but suffice it to say here that the objects created by the factory and the Foo object it self are bound by an external context that makes this binding required. The design we chose is really not the interesting part to talk about either.

The interesting parts was what else we learned in such a short time. In an intense competitive "get this done in 3 hours" setting, some important impediments got really visible:
  • Building the software took time - disrupting flow, and impeding progress.
  • Unittests that tested more than the unit (e.g. becoming integration tests) became a stumbling block (The test of the Foo class was strongly coupled to some of the concrete subclasses of Bar, and even tested it's behavior. This caused the refactoring to become painful and difficult.)
  • Lack of communication while pairing - contributing factor to not having a working solution in the end.
  • Making mock-objects for objects that inherits from QObject is actually a PITA.
These impediments made us spend more time "mucking about", than actually designing and implementing. We knew about them, of course, but this very short sprint made them so visible, that us actually doing something about them are more probable.

Finally: The code-off have a wanted side-effect. There is no doubt in the team, where the design of that particular class (and it's immediate surroundings) is going, reducing the cost when designing and implementing the next similar features.

The initial goals were achieved, we learned a lot just from this little four hours exercice. The ROI is definitely positive. We'll do it again.

There are several things that lends it self to "competition-form"; small refactorings like discussed herein, entire design spikes, UI design mockups and through to implementing an entire feature. I will be doing more of this in the future.