More Micro-optimizations
Here's another micro-optimization that I'm not very fond of. I should note that we don't really obsess about these things too much internally - in general we have established coding practices and only occasionally debate a few debatable things, usually on the fringes. But they do make reasonable blog fodder and I'm trying to cover ones that haven't been debated ad nauseam.
HRESULT DoSomething(HWND hwndParent, BOOL fFoo, BOOL fBar)
{
HRESULT hr;
if (hwndParent != NULL)
{
if (fFoo)
{
hr = DoOperationWithFoo(hwndParent);
}
else if (fBar)
{
hr = DoOperationWithBar(hwndParent);
}
}
return hr;
}
Catch the bug? This one's so obvious I'm not going to wait a day: if "hwndParent" is NULL then "hr" is left uninitialized. The micro-optimization here is that not initializing "hr" up front potentially saves a few instructions. The argument is that code size is one of the most critical factors for application performance on a heavily multitasking modern desktop (which is true; nearly all of our code is compiled for minimum size rather than maximum performance) and that this helps because it all adds up. The fix for the people who do not prefer initializing up front is to add "else" clauses to set "hr" to the correct error values. You may guess that with that fix you end up with a similar total number of instructions (or even more in this example), but in many cases initializing up front is certainly redundant.
The counter argument, of course, is that the risk of using uninitialized variables far outweighs the minor performance increase. This is the school of thought that I subscribe to.
The counter-counter argument is that modern compilers are likely to pick this up, and even if they don't our more advanced static analysis tools will.
The counter-counter-counter argument is that I've personally fixed around a dozen bugs in other people's code over the years that were the direct result of doing this, and it's also annoying when stepping through the debugger and seeing garbage values. :-)
For the most part this debate can be skirted by doing what I consider a C++ best practice:
HRESULT hr = DoSomething(hwndBlah, TRUE, TRUE);
That is, reducing the scope of variables as much as possible and avoid splitting initialization from assignment. However, in the above contrived example this isn't really possible.
