Saturday, March 12, 2011

Proportionality of incremental changes

A few weeks ago I was watching a presentation on Project Lambda. The proposed syntax is generally quite nice. It's very simple, and avoids a lot of the noise which pollutes typical Java programs using inner classes. But one of the syntax examples didn't sit right with me, and it took me a few days to figure out exactly why.

The particular example isn't important, and I don't think it's currently in the public proposal. But what is important, I realized, is that code should be written to facilitate incremental changes, and this interfered with that.

What do I mean?

Here's an example: one of the rules in our coding guidelines is that control blocks must always use curly brackets. That is, don't write code like this:

if (condition)
  doSomething();
else
  doSomethingElse();

We require that this code be written like this, instead:


if (condition) {
  doSomething();
} else {
  doSomethingElse();
}

Here's why I don't like the syntax without the curly brackets (at least one of the reasons): it makes it harder to modify the code.

If you want to add an extra line inside the else block, you need to add the line and convert the single statement block into a compound block:


if (condition)
  doSomething();
else {
  fprintf(stderr, "Debugging doSomethingElse\n");
  doSomethingElse();
}

To add one line of code I need to modify three lines! This is a disproportionate amount of work considering the actual change.

In summary, code should be written in a way which encourages incremental changes. If small logical changes require large textual changes, then there's something wrong, either with the tools or the technique.

Thursday, March 10, 2011

Overloading and varargs

I learned something important today: don't overload a variadic function.

It's very tempting to have functions like this in your C++ class:

class Foo {
public:
  void write(const char* format, ...);
  void write(const chat* format, va_list args);
}

The variadic function would be implemented like this:

void Foo::write(const char* format, ...) {
  va_list args;
  va_start(args, format);
  write(format, args);
  va_end(args);
}

It looks nice and clean -- a perfect application of overloading.

However there's a subtle problem! The C++ compiler will always try to resolve the method with the most specific signature when it encounters an overloaded call.

Calls like this are fine:

foo->write("%s\n", "Hello");
foo->write("...world");

But what if you have an argument which looks like a va_list? For example, if va_list is a pointer on your platform, what does this line do?

foo->write("How many chickens? Answer: %d\n", 0);

In C++ 0 isn't just a number. It's also the NULL pointer. The compiler will decide that you're actually calling the non-variadic function and will use NULL for the va_list argument. Then your program will crash when you try to read an int argument from a NULL va_list.

Lesson learned. Don't overload functions if one of the functions is variadic.

Now I have to go back and fix some code I wrote yesterday...