The Not-So-Old New Thing

Raymond Chen is one of the proverbial oldboys at MSFT. He runs this blog.

...he is apparantly famous enough to have his own wikipedia entry as well (http://en.wikipedia.org/wiki/Raymond_Chen), although then again the requirements for having your own wikipedia page aren't exactly breathtaking.

Okay, okay, not trying to be a curmudgeon, but I digress: Raymond is generally considered to be a very knowledgeable, somewhat prolific entity in the Win32 programming world. That being said, I think he made a very basic error in one of his recent blog posts:
http://blogs.msdn.com/oldnewthing/archive/2006/06/01/612970.aspx

The posting itself makes a very good point about the dangers of the CS_OWNDC class style. However, the following function is a poor example of quality code:

void FunnyDraw(HWND hwnd, HFONT hf1, HFONT hf2)
{
HDC hdc1 = GetDC(hwnd);
HFONT hfPrev1 = SelectFont(hdc1, hf1);
UINT taPrev1 = SetTextAlign(hdc1, TA_UPDATECP);
MoveToEx(hdc1, 0, 0, NULL);
HDC hdc2 = GetDC(hwnd);
HFONT hfPrev2 = SelectFont(hdc2, hf2);
for (LPTSTR psz = TEXT("Hello"); *psz; psz++)
{
POINT pt;
GetCurrentPositionEx(hdc1, &pt);
TextOut(hdc2, pt.x, pt.y + 30, psz, 1);
TextOut(hdc1, 0, 0, psz, 1);
}
SelectFont(hdc1, hfPrev1);
SelectFont(hdc2, hfPrev2);
SetTextAlign(hdc1, taPrev1);
ReleaseDC(hwnd, hdc1);
ReleaseDC(hwnd, hdc2);
}

...the manual, unprotected calls to GetDC and ReleaseDC are crude and error prone. When an application makes a call to GetDC (which generally shouldn't be done--most people using GetDC don't actually need to), it must be followed with a call to ReleaseDC. This is a pretty basic programming principle: if some object or resource is allocated by your program, you need to give it back at some future time.

My beef is with the manual calls to GetDC/ReleaseDC. If the above code used the CClientDC class, it would automatically call ReleaseDC when the declared CClientDC object went out of scope, thus removing the responsibility from the programmer.

Writing good, maintainable code is about more than making your code error-free; it's about bullet-proofing your code for future programmers who may not know better. So, your successor lumbers along, and decides to throw in a return statement midway through this call, alienating the HDC object--had Raymond originally used CClientDC, this wouldn't have been a problem.

Likewise, if the code throws an exception, the class automatically goes out of scope. Problem solved. Given that there is no error checking on the inputs (unforgiveable sin #2, but I'll leave that rant for another day), the possibility of an exception being thrown (and then caught somewhere up in the call stack) is pretty good.

When I presented this minor edit to Raymond, he had some terse, evasive comments to the effect of: "I think you missed the point of the article. CClientDC wouldn't have helped any." Well, duh. Of course it wouldn't have helped any--but that's not the point I'm making! The bigger observations here are: most of the time, using GetDC isn't appropriate--the very use of GetDC itself is suspect. Second, using objects that always have a requisite "cleanup" action without having that cleanup managed by the C notion of scope is code that isn't worth fresh cow dung.

When I clarified myself, I get yet another strangely evasive comment: "I already explained multiple times why my samples do not use any class libraries. Plain C is the common language. If you want to use a particular dialect then more power to you." Okay, fine: so write your own basic wrapper class (or merely copy the code for CClientCD, which is freely available), but for God's sake, stop proliferating crap programming practices on people who should know better!

MSFT has this strange habit of doing the above; it all reminds me of the DirectShow filter pointers fiasco. DirectShow filters are COM objects, and the vast, vast majority of all of the samples provided by MSFT use raw COM pointers. Needless to say, this is moronic: and because of it, the vast majority of DirectShow programs are, to use the parlance of our times, Seriously Fucked. The problem is programmers forgetting to call Release() on their COM objects, or having the program encounter an error/throw an exception, and not clean up after itself because a programmer didn't have appropriate error checking in one of several hundred places they'd need to put it. Most people end up using raw pointers because all of the sample code provided to them uses raw pointers. In other words, it's a "do as I say, not as I do" sort of situation--nobody well versed in the ways of COM would touch a raw COM ptr, but in this asinine push to "not use a particular dialect," they are actually just proliferating garbage code.

The solution to DShow woes is to use CComPtr. CComPtr protects you from this, because when a declared CComPtr object goes out of scope, it automagically calls Release() on the underlying COM object. The programmer no longer has to be concerned with it. To not use this sort of automatic, basic protection while programming is irresponsible at best, and demented at worst.