noah’s mark

I was randomly flipping through Code Complete today (on a slight whim of masochism), and read a whopping 2 pages before I came upon YASI (Yet Another Stupid Idea), for which this book is famous for (famous among people who prefer to think when they code, rather than crapping code out their, well, you know.)

On page 750, McConnell talks about avoiding gotos. Still, he says, if you have a circumstance where they might be useful, here are some nifty tips! My problem isn’t with the general advice, although in the vast majority of modern languages, goto is entirely unnecessary, and the languages in which it is necessary are somewhat limited (read: C, for example, is a not-very-powerful language).

Here is the “C++ example of making the best of a bad situation (using goto)”, which is Listing 31-34:

void PurgeFiles( ErrorCode & errorCode ) {
	FileList fileList;
	int numFilesToPurge = 0;
	MakePurgeFileList( fileList, numFilesToPurge );

	errorCode = FileError_Success;
	int fileIndex = 0;
	while ( fileIndex < numFilesToPurge ) {
		DataFile fileToPurge;
		if ( !FindFile( fileList[ fileIndex ], fileToPurge ) ) {
			errorCode = FileError_NotFound;
			goto END_PROC;
		}

		if ( !OpenFile( fileToPurge ) ) {
			errorCode = FileError_NotOpen;
			goto END_PROC;
		}

		if ( !OverwriteFile( fileToPurge ) ) {
			errorCode = FileError_CantOverwrite;
			goto END_PROC;
		}

		if ( !Erase( fileToPurge ) ) {
			errorCode = FileError_CantErase;
			goto END_PROC;
		}	

		fileIndex++;
 }

END_PROC:

	DeletePurgeFileList( fileList, numFilesToPurge);
}

Now you too can write really, really awful code. Just look at Steve! He even wrote a book full of crappy code!

All right, time for me to nit-pick a bit. Keep in mind, this code was written in C++, not C. Here are the minor problems:

• The function “returns” an error code, but it is a single reference variable; it should be returned, rather than being a void function. Sometimes you don’t have control over this, but when you do, your function should return what it returns (again, this isn’t C).
• Creating and deleting the FileList (which is an object, as you can see in the declaration) is hidden in obscure functions, rather than using new/delete. Also, FileList is an object that apparently has no concept of its own size, since the explitic numFilesToPurge is passed around everywhere. Again, you don’t always have control over allocators or deallocators, but, then again, C++ gives you the ability to define your own on a class-by-class basis. Again, this isn’t C anymore.
• Using a while loop where he really wanted a for loop. This he does have complete control over.
• The members of the FileList should probably be objects as well, that you can query about (are you writable? do you exist? are you open?)

And now, drum roll please:

Not understanding the concept of RAII.

I kinda wish Steve McConnell would read this blog, or maybe read up on C++, or maybe stop to think about what he is publishing. Then again, the book is Microsoft Press, and the quotes at the beginning are either from useless people or useless people who pretend to be part of the open source movement.

Anywhoo, the point is that, in this case, what he really means to do is say “Create this dynamically constructed list of files, and then make sure to delete it before the function exits.” Wouldn’t it be nice if he was programming in a language that has the ability to have scoped variables? Wouldn’t it be nice if he had a language where these scoped variables were deterministically destructed?

Hmm, if only.

Written in, well, C++, by somebody with at least 1/3 of a brain-cell, the code would look, at best, something like this:

ErrorCode PurgeFiles() {
	FileList fileList = MakePurgeFileList();
	for(int fileIndex = 0; fileIndex < fileList.length(); fileIndex++) {
		DataFile fileToPurge;
		if ( !FindFile( fileList[ fileIndex ], fileToPurge ) )
			return FileError_NotFound;
		if ( !OpenFile( fileToPurge ) )
			return FileError_NotOpen;
		if ( !OverwriteFile( fileToPurge ) )
			return FileError_CantOverwrite;
		if ( !Erase( fileToPurge ) )
			return FileError_CantErase;
	}
	return FileError_Success;
}

Using the magic of, well, taking a few seconds to think about what I’m doing, I’ve managed to take a file that was 29 lines and make it into a file that is 15 lines. In case you weren’t paying attention, here are the changes:

• fileList is constructed where it was constructed in the first place. I can only assume that, in the first example, FileList is a typedef for a pointer-type (Survey says? WRONG. Be extremely careful about making typedefs for pointer types, as it confuses how the type is to be allocated and deallocated. After all, saying “FileList fileList;” sure looks like a stack-allocated variable to me)
• There is no errorCode variable being set all over the place. When something bad happens, that error code is returned. If the method makes it all the way to the end, then it returns successful. This is a common enough paradigm that you should be able to pick up on the exit points to the code (on a related note, the people who tell you, “You should only have one return statement” [Jesse Liberty, the world's second worst computing author, I'm talking to you!] are full of crappity crap crap).
• There is no goto, because all of the clean-up code happens automatically, since FileList is stack allocated and all (NOTE: this doesn’t mean the underlying structure is stack allocated, but it does mean that the destructor will definitely be called. Think of the standard C++ collections, like vector. You don’t worry about deallocating a vector, even though the underlying memory is all dynamically allocated).
• The while loop is now the much cleaner for loop. Just think about how ugly the original would have been if you had any continue statements in there. I’m sure Steve would have just written another goto for that, too.

The biggest difference, though, is that the second example looks like C++. The first example looks like C, and is decent C. In C, we didn’t have some of these nice concepts. This should be readily apparent, even at casual viewing.

So here’s the kicker. When you read through Code Complete (never, ever read it, though. This is just hypothetical), the C++ examples, for the most part, should strike you as wrong in some way. I don’t mean wrong like they produce errors, but wrong as in they don’t quite smell right. Something about them should just make you feel uncomfortable.

Maybe you should read the book, in the hopes of getting a better idea of what I mean. Code written in this fashion, which is generally along the lines of writing assembly or C code in C++ (or any language, for that matter). As a general rule of thumb, your code should use whatever your language offers you. In C#, the example could be cleaned up similarly by using using(FileList fileList = MakePurgeFileList) {, which would deterministically dispose of the object when the using block was exited. Other languages give us similar capabilities.

And the final goto rule is? Probably only in C, but even then, rarely. Then again, if it fits somewhere in your C++ code, then use it in your C++ code. The real goto rule is just a smaller part of the overall rule, which is this:

Stop writing crappy / ugly / overly-complicated / wrong-language / Steve-McConnell-esque code.

This entry was posted on Monday, July 9th, 2007 at 10:35 pm and is filed under general. You can follow any responses to this entry through the RSS 2.0 feed. Both comments and pings are currently closed.