HiveBrain v1.2.0
Get Started
← Back to all entries
principlecsharpMinor

Concise null checking vs readability

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
nullreadabilitycheckingconcise

Problem

I've got a case where I need to call a couple of methods, checking if they return null, and return some property from the result of the second when neither returns null.

I've thought of two ways to do this - one "verbose" and one "concise". For the purposes of this question, I'll just call the return types 'dynamic' since I'm not interested in those details here.

private dynamic VerboseVersion()
{
     var a = CallMethodA();
     if ( a == null ) 
     { 
         return null; 
     }

     var b = CallMethodB( a );
     if ( b == null ) 
     { 
         return null; 
     }

     return b.SomeProperty;
}

private dynamic ConciseVersion()
{
    dynamic a;
    dynamic b;

    if ( ( a = CallMethodA() ) == null ||
         ( b = CallMethodB( a ) ) == null )
    {
        return null;
    }

    return b.SomeProperty;
}


I the first because the intent at each step is very obvious but I find it somewhat verbose; I like the second because the intent of the function seems more obvious and it is less verbose but it may be a bit harder to follow at first.

Thoughts?

Update: Corrected a typo and corrected the formatting of the "Verbose" version to what I would actually use in committed code.

Solution

I take great pleasure in the fact that your "concise" version is longer than your "verbose" version!

If you believe one version to be more readable and the other to be more obscure, I would argue going with readability over obscurity. One obscure detail is that you're relying on people understanding

(a = CallMethodA()) == null


Some people might think that to be an error, not realizing the expression on the left actually has a value.

If you want to use less lines of code, you could go with something like this

var a = CallMethodA();
var b = (a == null) ? null : CallMethodB(a);
return (b == null) ? null : b.SomeProperty;


Which isn't altogether different than your original if/else chain, but is a bit more compressed and has just the one return statement.

Code Snippets

(a = CallMethodA()) == null
var a = CallMethodA();
var b = (a == null) ? null : CallMethodB(a);
return (b == null) ? null : b.SomeProperty;

Context

StackExchange Code Review Q#3810, answer score: 8

Revisions (0)

No revisions yet.