Fabulous Adventures In Coding
Eric Lippert is a principal developer on the C# compiler team. Learn more about Eric.
I'm insanely busy prepping for VSLive! today, so, once more, I complain about coding practice that are drivin' me nuts.
But first: why do pirates dislike Johnny Depp's Oscar-nominated performance in "Pirates of the Caribbean"?
Because it wasn't AAAAAARRRR-rated.
Gripe #3: More Complaining About Bad Macros
One day I was debugging a bunch of macro-ridden code that I didn't write. I wanted to know what the method signature of
Inspiration hit -- I grepped for
DEFINE_DERIVED_GETSET_METHODS(CForm, CServer, MouseIcon, IID_IPicture)
Aaargh! I still didn't know what the signature was until I managed to find that macro definition and decode it! Why do people write code like this? You can't easily debug into
Gripe #4: Exception Handling, Return Values and Smart Pointers
You probably knew I couldn't actually not complain about smart pointers,
Before I start complaining about exception handling, please, I don't want to have a replay of that Joel On Software debate about the merits of structured exception handling. I think exception handling is super. I love writing code in C# that uses exception handling.
The problem I have with structured exception handling is that programs that mix libraries that use SEH and libraries that return error codes, (like, say, every single COM API) often turn into a godawful mess. You end up with the worst of both worlds: exceptions potentially going off all over the place acting as non-local gotos, and still having to pay careful attention to error return codes, thereby de-localizing the error handling code. C# and the .NET Framework were designed to use exception handling from day one, so it makes a lot of sense. COM was not!
As we saw
Case in point -- one of my coworkers and I were debugging a weird problem in some of our code just last week. We called into a particular API which was setting the
// Ensure that we've preserved the error state we captured earlier SetLastError(blah); return hr; }
The caller of this method calls GetLastError , but the state we just set has been blown away. OK, smart people, where does that happen?
Turned out that a smart pointer in the current scope was holding on to a proxy for an out-of-process COM object, and when the smart pointer went away, the out-of-proc object was released, which resulted in a whole slew of calls behind the scenes, which resulted in the
} <-- calls destructor
Aargh! It's drivin' me nuts! Call me crazy, but I want operations bearing complex semantics and causing side effects that break my error handling to look a little more like function calls and a little less like punctuation. Had the code been written using dumb pointers, no problem
// Ensure that we've preserved the error state we captured earlier SetLastError(blah); pProxy->Release(); return hr;} <-- no op
Now it's obvious where the mistake is.
COM programming demands discipline and a keen understanding of how to write correct error-path code whether you also use exception handling or not. Therefore, I say don't use exception handling if you're going to be doing a whole lot of COM-style error handling too! I like to think that I'm a fairly bright guy, but I have great difficulty analyzing the semantics of code that has macros, template classes, smart pointers, exception raisers, error return values and cross-process COM calls all in the same routine. I'm not that smart!
I've developed a coding style that works very well with COM -- it is verbose, it is rigid, and it is very, very clear to even novice programmers exactly what the lifetime of all pointers in the code are -- and my code rarely has reference leaks because of it, even in error cases. And when it does, they are a snap to track down because every time a pointer is addrefed there is a call to AddRef and every time it is released there is a call to Release right there in the code, not tucked away in some template library. It's not hard to develop a good coding standard.
Next time: even more on bad interactions between exception handling and memory management
Why not do this:
} <-- This will call the destructor
} <-- no-op
I think that your main Problem is that you use global Variables as return values as you do in SetLastError(). Just see which trickery glibc needs to make errno threadsafe, and you'll know how evil it is.
There are several inventions in most modern languages, and each of them makes SetLastError & co obsolete:
- structs / tuples as return parameters
- out parameters / call by reference
The only excuse for using global variables to return results is when you use ancient C libraries, and then you best wrap those libs using well-designed functions.