Return-code vs. Exception handling

(Originally authored Feb 2008.  This is a revision of an older post from before I began blogging on the internet at large.  It’s been edited for style, not content)

This is one of those “religous wars” in programming language theory; return code handling (RCH) vs exception handling (EH).  Firstly, I am biased.  Secondly, I will try to remain objective as well, partitioning observations of code using these mechanism from how I feel this reflects on the utility of either mechanism.  I tend to _prefer_ EH in general, but not universally, and generally follow the viewpoint that “exceptions are for exceptional things”.  With that aside…

Raymond Chen wrote an article a while back on RCH vs EH^[2]. There, he points out accurately some facts about code review of RCH and EH, concluding that it’s generally harder to conclude the failure-safety of some example the RCH code than some comparable EH code.

He’s right.  And he’s wrong.  His article is incomplete.  Lets start with defining the playing field.

Return Code Handling

Below, we have two versions of RCH-style code. All failures are communicated via return code. The left version is an erronous example, the failure paths have not yet been considered. The right version has corrected this fault.

BOOL ComputeChecksum(LPCTSTR pszFile, DWORD* pdwResult)
{
    HANDLE h = INVALID_HANDLE; 
    HANDLE hfm = INVALID_HANDLE; 
    void *pv;
    DWORD dwHeaderSum;

    h = CreateFile(...);

    hfm = CreateFileMapping(h, ...);

    pv = MapViewOfFile(hfm, ...);

    CheckSumMappedFile(... &dwHeaderSum, pdwResult);

    UnmapViewOfFile(pv);
    CloseHandle(hfm);
    CloseHandle(h);
    return TRUE;
}

BOOL ComputeChecksum(LPCTSTR pszFile, DWORD* pdwResult)
{
    HANDLE h = INVALID_HANDLE; 
    HANDLE hfm = INVALID_HANDLE; 
    void *pv = NULL; 
    DWORD dwHeaderSum;
    BOOL result = FALSE;

    if ( (h = CreateFile(...)) == INVALID_HANDLE)
        goto cleanup;
    if ( (hfm = CreateFileMapping(h, ...) 
          == INVALID_HANDLE ) )
        goto cleanup;
    if ( (pv = MapViewOfFile(hfm, ...)) == NULL )
        goto cleanup;

    CheckSumMappedFile(... &dwHeaderSum, pdwResult);
    result = TRUE;

    cleanup:
    if (pv) UnmapViewOfFile(pv);
    if (hfm != INVALID_HANDLE) CloseHandle(hfm);
    if (h != INVALID_HANDLE) CloseHandle(h);
    return result;
}   

The left side looks very clean, and very clearly does no RCH, so according to Chen’s method it is clearly incorrect. According to [2], the left one is ‘Really Easy’ to write, and ‘Really Easy’ to recognize as being incorrect. The lack of an if-statement every 2 lines is a red flag. Basically, you must explicitly write every possible success and failure code path, even the “uninteresting” ones.

Let’s move on to the EH example from Chen’s post.

Exception Handling (old style)

Bellow is adaptation of both the incorrect EH code from his article, along with a possible fix.

NotifyIcon* CreateNotifyIcon()
{
    NotifyIcon* icon; 

    icon = new NotifyIcon();

    icon.set_text("Blah blah blah");
    icon.set_icon(new Icon(...), GetInfo());
    icon.set_visible(true);

    return icon;
}

NotifyIcon* CreateNotifyIcon()
{
    NotifyIcon* icon = 0;

    icon = new NotifyIcon();
    try {
        icon.set_text("Blah blah blah");
        icon.set_visible(true);
        Info info = GetInfo();
        icon.set_icon(new Icon(...), info);
    } catch (...) {
        delete icon; throw;
    }
    return icon;
}

I totally agree with any and all critics that the above style of code is tough to diagnose. If any of { set_text, set_icon, set_visible, or get_info } can throw, the left code is totally busted. The code to the right is tricky… by moving GetInfo to a separate line, and set_icon to the end, the Icon object can’t be leaked (presumably set_icon is responsible for freeing the Icon in failure conditions). The try-catch will delete NotifyIcon and propogate the exception, but not without having made this program a pain to debug – we will see two exception raise events now instead of just one.

Even worse, tomorrow this code could become incorrect again – if the copy-constructor for Info can throw, the right side code is still wrong. I bet you didn’t think of that. Think you’re an expert at this? Try item #18 in Exceptional C++^[3]. I think my first time I found about 16 code paths in the 3 lines of C++ code, barely teetering into ‘Guru’ category, but I spent like 15 minutes on it. Normally you have 5 seconds during a code review, and you won’t find all 23 code paths in 5 seconds. You have to do better than analyzing ever code path.

Applying Raymon Chen’s categories, getting to the correct code is “Really Hard”, and I hope the audience agrees with me. It is hard. Any line of code, any subexpression could throw, and we have to look at all those possible cases. RCH certainly looks better. Sure, we’d have to have like 9 ‘if’ statements to bring the above code in line, but at least then we could see that we covered all the code paths. But there is no general rule we can apply to see if it is trivially incorrect. In RCH, a line without a conditional branch is suspect. In EH (old style), a line without a try-catch after resource acquisition is suspect.

I call this ‘old style’ because it looks a lot like how you’d handle exceptions in C#, or in C++ without any library support back in 1990. Stephan T. Lavavej makes a distinction between “archaic C++” and “modern C++”^[4]. And Raymond Chen notes in parting words, “Yes, there are programming models like RAII and transactions, but rarely do you see sample code that uses either.” – let’s remedy this situation.

Exception Handling (modern)

On the left, we have some bad RAII exception handling code, to parallel Chen’s original bad example. On the right, the ‘fixed’ version.

shared_ptr<NotifyIcon>
CreateNotifyIcon()
{
    NotifyIcon*
      icon( new NotifyIcon() );

    icon->set_text("Blah blah blah");

    icon->set_icon(
        shared_ptr<Icon>( new Icon(...) ),
        GetInfo());

    icon->set_visible(true);

    return icon;
}

shared_ptr<NotifyIcon>
CreateNotifyIcon()
{
    shared_ptr<NotifyIcon> 
        icon( new NotifyIcon() );

    icon->set_text("Blah blah blah");

    shared_ptr<Icon> 
        inner( new Icon(...) );
    icon->set_icon(inner, GetInfo());

    icon->set_visible(true);

    return icon;
}

I’m changing the rules. I don’t care how many branches there are, success or failure branches. What I do care about is that, at every point of time, there is exactly one owner for ever resource, and that in a general failure condition, that owner will take care of the resource.

That is the fundamental change. Modern C++ code using RAII does not generally have any explicit code for failure conditions. General failures are always handled non-locally. Local resource management is immediately handled by automatic objects, using RAII. Any raw resources (such as allocating a new object) must immediately be handed off to an RAII object.

Lets take a closer look at what I find wrong with the left example.

• First line, we have a raw pointer. Bad. RAII rule: raw pointers == suspect code.
• Second line, set_text. No raw resources. Good.
• Third line, shared_ptr is mixed in with another call. Bad. Move to a separate line.
• Last line, set_visible, also good.

How do we know we are done? When there are only two kinds of statements – RAII object initialization, and useful code that doesn’t create unadorned resources. Now, it is admittedly hard to get some RAII classes right in the first place, but that only needs to be done once per class, instead of once per use.

While it is still hard to see all the code paths, it becomes really easy to check if the developer has finished EH. Any resource allocation without RAII will tell you that the developer didn’t finish. Sure, bad RCH code pops out like a rose in a weed garden, but with RAII you can usually skip writing the bad code in the first place – just jump directly to shared_ptr<T>, or shared_factory<HANDLE, close_handle>::type, or scope_guard<T>, etc. You can skip writing the bad code at all, and do all your failure handling via RAII.

Final Thoughts

So, we have a new table for recognizing code:

And here’s how I believe modern EH with RAII fits in Raymond Chen’s table.

Finally, I’ll leave you with my interpretation of RCH vs. EH (modern).

HRESULT
CreateNotifyIcon(NotifyIcon** ppResult)
{
    NotifyIcon*      icon = 0;
    Icon*            inner = 0;
    const wchar_t *  tmp1 = 0;
    HRESULT          hr = S_OK;

    if ( SUCCEEDED(hr) ) {
        icon = new (nothrow) NotifyIcon();
        if ( !icon ) hr = E_OUTOFMEM;
    }

    if ( SUCCEEDED(hr) )
        hr = icon->set_text("Blah blah blah");

    if ( SUCCEEDED(hr) ) {
        inner = new (nothrow) Icon(...);
        if ( !inner )
            hr = E_OUTOFMEM;
        else {
            Info info;
            hr = GetInfo( &info );

            if ( SUCCEEDED(hr) )
                hr = icon->set_icon(inner, info);
            if ( SUCCEEDED(hr) )
                inner = NULL;
        }
    }
    if ( SUCCEEDED(hr) )
        hr = icon->set_visible(true);

    if ( SUCCEEDED(hr) ) {
        *ppResult = icon;
        icon = NULL;
    } else {
        *ppResult = NULL;
    }

    cleanup:
    if ( inner ) delete inner;
    if ( icon ) delete icon;
    return hr; 
}

shared_ptr<NotifyIcon>
CreateNotifyIcon()
{
    shared_ptr<NotifyIcon> icon;//[5]
    shared_ptr<Icon>       inner;

    icon.reset( new NotifyIcon() );
    icon->set_text("Blah blah blah");

    inner.reset( new Icon(...) );
    icon->set_icon(inner, GetInfo());
    icon->set_visible(true);

    return icon;
}

Footnotes:

1 – C++ FAQ Lite, Section 17 “Exceptions and error handling” http://www.parashift.com/c++-faq-lite/exceptions.html

2 – Raymond Chen, “Cleaner, more elegant, and harder to recognize” http://blogs.msdn.com/oldnewthing/archive/2005/01/14/352949.aspx

3 – Herb Sutter, “Exceptional C++”, http://www.amazon.com/Exceptional-C%2B%2B-Engineering-Programming-Depth/dp/0201615622/

4 – Stephan T. Lavavej, aka STL, “Modern C++” http://nuwen.net/14882.html

5 – some may note that auto_ptr (or unique_ptr in C++0x) may provide better efficiency, and safely auto-converts to shared_ptr. That is another alternative.  Just don’t use raw ptrs and make sure to review Effective C++ on auto_ptr safety.