Today we were fixing a minor bug in the installation procedure of Code Co-op. Since our development methods are very different from most other people’s, I though it would be worth blogging about it.

You might say we do some weird version of Extreme Programming–not because we decided to do so, but because it naturally evolved in our development environment. You must know that I work from Seattle, WA, and Wieslaw works from Ireland. We use Skype and Mikogo to do pair programming.

Here’s a short description of the bug: The installer has two execution paths, depending on whether Code Co-op has already been installed (and this is an update), or it’s a fresh installation. However, it doesn’t deal very well with a situation when Code Co-op has been partially uninstalled.

In particular, if the database path doesn’t exist in the registry, we ask the user to provide one. But if the path exists, we use it blindly, without verification. That’s not good!

In order to fix this bug, we needed to verify the database path obtained from the registry before proceeding any further. We noticed that such verification code already existed in the dialog box that asks the user for the path. We had several choices:

  1. Copy and paste. Copy the code from the dialog into the installer. We don’t do that at Reliable Software!. The reasons are obvious, and I’m mentioning this option only because copy-and-paste is notoriously abused by many, otherwise fine, development teams.
  2. Abstract (refactor) the verification code so it can be called in both places.

Interestingly, the code was already partially abstracted. Our whole dialog machinery is pretty thoroughly refactored.

In particular, in our library, every dialog has its own “model” object (often called “dialog data”). If you don’t remember what the Model-View-Controler pattern is, let me just point out that the Model is independent of the UI. So we have a specialized “model” object for our database-path-querying dialog. Since it is created in the install procedure,we already have access to it at the crucial point. And it has the Validate method that we needed so badly outside of the dialog. We can use this object to validate the path from the registry and the problem is solved!

Now, there is a little hiccup: In the dialog, the validation routine displayed errors using message boxes.

When I said that the Model is UI independent, I lied. We decided some time ago that it was just too inconvenient not to be able to pop message boxes from inside model objects. So now we have this global object called TheOutput, through which we display messages to the user.

As usual, we had options:

  1. Split the validation function into two–one doing the check, the other displaying the errors. We do it in several other dialogs, but it either requires custom encoding of errors, or substantial code duplication.
  2. Pass the validation routine a “quiet” Boolean flag that would suppress message boxes

We settled on the latter.

Now for the mechanics of suppressing the output. The first thing that comes to mind is to put all the output under if(!quiet) clauses. This would work, but it’s messy, especially if you have many branches displaying errors. There is another option though. TheOutput object has an on/off switch–we could conditionally turn it off for the duration of the validation, and then turn it back on.

Again, the first solution that comes to mind is wrong. Putting TheOutput.SetVerbose(false) at the entrance, and TheOutput.SetVerbose(true) at the exit of the function is wrong for several reasons.

  1. We should restore the previous value of “verbose” at the exit, not set it to true. The output might have been already turned off.
  2. The code is error prone–you have to make sure the verbose flag is reset before every return statement (and that includes future modifications to the flow of controll).
  3. This solution doesn’t work when an exception is thrown–and we do use exceptions to propagate serious errors. (BTW, everybody who calls new uses exceptions

This particular constellation of concerns happens so often that we don’t even analyze it any more. We know that it’s another case for RAII. (This ugly acronym has nothing to do with prosecuting children for listening to music on their computers. It stands for Resource Acquisition Is Initialization.)

In that particular case, we just had to define an object whose constructor retrieves and stores the value of the verbose flag from the output, and turns the output off. Its destructor, in turn, restores the previous “verbosity” of the output. It’s a no-brainer and it solves all the above concerns.

class Muter
{
public:
    Muter(Out::Sink & output, bool quiet)
      :  _output(output), _verbose(output.SetVerbose(!quiet))
    {}
    ~Muter() { _output.SetVerbose(_verbose); }
private:
    Out::Sink & _output;
    bool _verbose;
};

bool DatabaseDlgData::Validate (bool quiet) const
{
    Out::Muter muter(TheOutput, quiet);
    ...
    // muter destructor called automatically, even upon exception
}

As always, when using RAII we have to make sure that the destructor of Muter cannot throw and exception.

It took us two hours to analyze and fix the bug, half of it spent fighting with Skype (it turned out that the latest release of Skype broke the Mikogo plugin, and we had to download and install standalone version of Mikogo, wait until our passwords were emailed to us, and then jump through several more hoops, before we could see each other’s screens).

Considering the elegance and the soundness of the fix, I’d say it was time well spent.