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:
1 2 3 4 5 6 7 | enum PaddleType { kLeftPaddle, kRightPaddle };class Paddle {public: Paddle(const PaddleType& type) : mPaddleType(type) { }private: 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:
1 | bool CollidesWith(const Paddle& paddle) |
Still looking reasonable, until one considers what happens when you do this:
1 | 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.
1 | 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.