Here is another one:

/////////////////////
LPCTSTR cpString = _T( "This was some string message..." );
//
// Later On In Code...
//
CString str( cpString );
char cChar = str.GetAt( 0 );
/////////////////////

If you write code like this, stop now and back slowly away from the keyboard – You’re Doing It Wrong!

The developer here is using a string object for a very simple operation. This is the kind of think people talk about when they say something like “using a shotgun to kill a fly”.

Extracting characters from a string (an array!) is a very basic operation – it is something we learn in our first C/C++ class or read about in our first C/C++ book. This is not something that you need a heavyweight class to help you out with.

Extracting the first character from cpString is as easy as doing one of the following:

/////////////////////
char cChar = *cpString;
//
// -Or-
//
char cChar = cpString[ 0 ];
/////////////////////

Remember – constructing and initializing an object always takes longer (i.e. has more overhead) than not constructing and initializing one. Think about wether or not you really need an object before you create one. If you can get along without it, see if doing so improves things.

For reasons mentioned in a previous post, in this case, the code is better without the CString.

This is the first of many examples of ways to misuse and/or abuse MFC’s CString class. While this example (and following ones) are specific to MFC, they likely apply to all string classes (mutable or not). Here is the offending code:

/////////////////////
CString str( "First Part Of Message\n" );
str = str + "Second Part Of Message\n";
str = str + "Third Part Of Message";

MessageBox( str );
/////////////////////

If you write code like this, stop now and back slowly away from the keyboard – You’re Doing It Wrong!

First, the developer is adding (concatenating) strings together, but these are static/constant strings! They always add up to the same string, and as such can be made into a single constant string:

/////////////////////
"First Part Of Message\nSecond Part Of Message\nThird Part Of Message"
/////////////////////

So at a minimum, the start of the code should read:

/////////////////////
CString str( "First Part Of Message\nSecond Part Of Message\nThird Part Of Message" );
/////////////////////

Why not add up the strings separately like the original code did? Two reasons – overhead and exception opportunity. Each use of CString::operator+(…) can result in dynamic memory operations (allocation and deallocation). So you are looking at six potential heap operations (three potential allocations and deallocations including destruction, although in release builds of CString, the number of operations is less). Each operation has the potential to raise an exception and in the absence of per-thread heaps, can effectively bottleneck a multi-threaded application to the performance of a single-threaded one because the heap operations have to be serialized.

So by manually putting the strings together we have reduced heap operations from 6 to 2 – one allocation and one deallocation. That is a pretty good improvement, but we can do better!

The MessageBox(…) function does not take CStrings, it takes pointers to constant strings (LPCTSTR). So why is a CString needed here at all?

/////////////////////
MessageBox( "First Part Of Message\nSecond Part Of Message\nThird Part Of Message" );
/////////////////////

This final version of the code is simpler, will execute faster, and is more robust. Sounds like a winner to me!

Note: Some of you may be thinking about the preprocessor’s ability to automatically concatenate static strings. Yes it does, but it cannot automatically coalesce the above strings because they are separate – they are being passed (separately) as parameters to a function. If the + operator was not present in-between the parts of the string, they would be coalesced to a single string, but the unnecessary CString would still be there.

Having been inspired by the amount of photos on the internet showing various forms of spectacular failures (http://www.doingitwrong.com), ranging from failed bunny-hops to the most graceful faceplants, I thought that a coding-equivlent of it might be worth trying out.

To that end, this section will cover little snippets of “wrong” code found in the wild. Unlike the photos, where the failure is usually fairly obvious, the failures present in the code snippets are not always as obvious, so a small discussion explaining the failure will always be present.

OK – enough of the BS…! Let’s get started!