[Kde-pim] if else construct detecting errors

[Ingo Klöcker]

On Monday 25 November 2013 13:04:33 Sandro Knauß wrote:
> Hey,
> 
> I wanted to know if there is a rule how to handle else constructs,
> when detecting errors. There are two cases to handle them. One is to
> use an else clause and the other is to exit the function directly in
> the if and stops futher processing.
> 
> IMHO the version to return directly is better readable and
> understandable. If I see a return i know, ok, now the function
> returns. If there is this big else clause behind, i have to look
> behind it to see if there is anything.

As Sergio wrote, there is no rule/convention for this.

I mostly follow those personal guidelines:
* I use early exits for sanity checks at the begin of a function. My 
reasoning for this is to keep the indentation level low.
* I avoid early exits for everything else.
* IMHO early exits are a no-go in case something has to be done in any 
case at the end of a function (as in the example you gave).


Some people, e.g. Robert "Uncle Bob" C. Martin, would argue that the 
question becomes moot if you make all functions as short as possible 
because then each function will consist of at most one control statement 
with single-line statements in its body/branches.

Uncle Bob would probably rewrite your example as follows (in pseudo-
code); the function names are probably not the best:

function setErrorTextFromUnicodeString( const QString &unicodeString )
{
    setErrorText( QString::fromLocal8Bit( unicodeString ) );
}

function setErrorCodeAndText( const ErrorType &error )
{
    setError( error.code() );
    setErrorTextFromUnicodeString( error.asString() );
}

function handleError( const ErrorType &error )
// a better name would say how the error is handled; "handleError" is 
not self-explanatory
{
    // [...] Removed
    setErrorCodeAndText( error );
}

function composeSignedBodyPart( ... )
{
    QByteArray signatureHashAlgo = res.createdSignature( 0 
).hashAlgorithmAsString();
    d->resultContent = MessageComposer::Util::composeHeadersAndBody( d-
>content, signature, d->format, true, signatureHashAlgo );
}

function handleErrorOrComposeSignedBodyPart( ... )
{
    if ( res.error() ) {
        handleError( res.error() );
    } else {
        composeSignedBodyPart( ... );
    }
}

function handleSignJobResult( ... )
{
    handleErrorOrComposeSignedBodyPart( ... );
    emitResult();
}


At first this looks extreme, but this approach has a few advantages 
(provided the functions and variables have good, self-explanatory 
names):
* Comments are almost entirely unnecessary.
* Each function is as readable as it gets.
* Separate Levels of Abstraction - If I just want to get a quick 
overview, then I just read the high-level function 
(handleSignJobResult); if I want to know the details, e.g. how is the 
signed body part composed, then I dive into a deeper level of 
abstraction (without being bothered how the error handling work in 
detail).
* The argument for using curly braces for one-line bodies of control 
statements becomes moot because there are no multi-line bodies for which 
the curly braces are essential.


Sorry, I digressed a bit. :-)


Regards,
Ingo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20131125/16884447/attachment.sig>
-------------- next part --------------
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/