return statements inside a function

Question

I was wondering how people handle return statements in a function. I have a method that allocates some memory but has a return false; statement when a function goes wrong. this statement is located about half way through the function so my memory is leaked if that function fails. This isn't the only return ...; statement I have in this function. What would stackoverflow recommend doing to clean up the code in a function with a few return statements in it?

if( true == OpenFtpConnection() )
{
    AfxMessageBox( _T( "Connection to internet and ftp server found" ) );

    // set the directory to where the xml file lives
    if( false == FtpSetCurrentDirectory( m_ftpHandle, _T(_FTP_XML_LOCATION) ) )
        return false;

    HINTERNET xmlHandle = NULL;
    WIN32_FIND_DATA fileData;
    SYSTEMTIME fileWriteTime;
    xmlHandle = FtpFindFirstFile( m_ftpHandle, _T("TPCFeed.xml"), &fileData, INTERNET_FLAG_RELOAD, 0 );
    if( NULL == xmlHandle )
        return false;
    else
    {
        // get the write time of the ftp file
        FileTimeToSystemTime( &fileData.ftLastWriteTime, &fileWriteTime );

        // get the write time of the local file
        HANDLE localFileHandle = NULL;
        localFileHandle = CreateFile( _T(_XML_FILENAME_PATH), FILE_READ_ATTRIBUTES,
                                 FILE_SHARE_READ, NULL, OPEN_EXISTING,
                                 NULL, NULL );
        if( INVALID_HANDLE_VALUE == localFileHandle )
        {
            AfxMessageBox( _T( "opening file failed, file not found" ) );
            return false;
        }
        else
        {
            CloseHandle( localFileHandle );
        }


        // close the FtpFindFirstFile() handle
        InternetCloseHandle( xmlHandle );
    }


    // download xml file to disk
    //if( false == FtpGetFile( m_ftpHandle, _T("TPCFeed.xml"), _T(_XML_FILENAME_PATH), FALSE, 
    //                       FILE_ATTRIBUTE_NORMAL, FTP_TRANSFER_TYPE_BINARY, 0 ) )
    //  return false;

}
else
{
    AfxMessageBox( _T( "No Connection to internet or ftp server found" ) );
    return false;
}
if( true == CloseFtpConnection() )
    AfxMessageBox( _T( "Connection to internet closed" ) );
else
    AfxMessageBox( _T( "Connection to internet not closed" ) );

Answer 1

Clarity is the most important thing. Many times, having multiple return points can make the program flow less obvious and thus more prone to bugs; but on the other hand, sometimes early returns are quite obvious; their meaning and purpose is clear. So I tend to avoid any hard rules about that.

You might get more mileage if you actually post the code you want to clean up though.

Answer 2

We've recently switched from the "one return per method" style to "return where it makes sense". Part of that switch was that we limit the number of lines in our methods to something reasonable (say 50 lines). By limiting the function size, the code becomes much more readable, and the multi-returns are natural, readable, and performant.

Answer 3

You haven't specified your programming language. Assuming it is C++: Use Boost's Smart Pointer. This not only handles multiple returns, but also exceptions thrown during method execution. If using Boost is not an option, it should be easy to create your own smart pointer class :-)