Explicit Constructors

I had a bug very recently that took much longer to track down than it needed to.  Actually, it should never have been a bug.  I’ve updated my coding standards accordingly and will make my best efforts to never allow this sort of bug to happen in the future.

I was making Pong, and had some code similar to the following:

enum PaddleType { kLeftPaddle, kRightPaddle };
class Paddle {
  Paddle(const PaddleType& type) : mPaddleType(type) { }
  const PaddleType mPaddleType;

At first glance this seems like acceptable code, it works, and does everything it is supposed to. Now consider we have a function that takes a Paddle as an input:

bool CollidesWith(const Paddle& paddle)

Still looking reasonable, until one considers what happens when you do this:

if (CollidesWith(kLeftPaddle))

For many the thought is “this shouldn’t compile, you’re passing a PaddleType as a Paddle”, it was actually my first thought when I finally found my bug, but then I realized what had happened…

When making a constructor with a single argument, the compiler will implicitly cast and create a temporary object. In few situations this is actually desired, but in my experience this desired effect is rare. More often is the case where you don’t want a temporary object created, what if resources are grabbed or the initialization is heavy on processing time? The solution is rather simple.

Use explicit constructors.

explicit Paddle(const PaddleType& type) : mPaddleType(type) { }

Making any single parameter constructors explicit will guard against any sort of behavior like this, instead of passing a temporary Paddle object into the CollideWith, the compiler would have thrown an error calling me an idiot. I prefer that. While I temporarily feel like an idiot for making a typo or mistake, it can prevent hours of debugging.

And so I’ve modified my personal Coding Standards to include; Always use explicit constructors for single argument constructors unless there is an explicit, and good, reason not to.

Comments are closed.