else or not else?

Hi all!

Just a little question. Should I use rather

if (cond) return a;
else return b;

or

if (cond) return a;
return b;

What's the actual difference at runtime

What do you prefer Thanks for any hints!



Answer this question

else or not else?

  • Alik Khavin - MSFT

    Well that's ok, but I'm thinking about really simple situations. The reason I went back to look if somebody post an answer was writing this function:

    public bool CanInvoke(short actionID)
    {
    DataRow r = actions.Rows.Find(actionID);
    if (r == null) return false;
    else return (!actionsDone.Contains(actionID) || (bool)r["Repeatable"]);
    }

    What I would like to know is, whether I should put the red else inside.

    I mean .... eg.. ...is this one useless instuction more in the MSIL


  • Rich609

    Hi Matthew,

    thats it, I prefer to use it the same way you described.

    However I believe my question was quite much simpler... the difference of in/excluding the red else..


  • Dan Krause

    > and as cgraus suggests, we should return only once in our code. it increases readability (not performance).

    I don't believe this always to be the case! You have to be pragmatic about this; dogmatically changing your code so that each function has only one return statement can result in some very hard to read code.

    It's particularly awkward when you are doing a lot of error checking at the start of a function, and instead of returning an error code immediately, you set a return value and an error flag, and then check that error flag repeatedly through the rest of the function. Or - just as bad - you start nesting the code.

    For example, here's a method that uses nesting to cope with error handling:



    public int DemoMethod1(string param)
    {
    int result = ERROR_NONE;

    if (Check1(param))
    {
    string something1 = DoSomeWork1(param);

    if (Check2(something1))
    {
    int something2 = DoSomeWork2(something1);

    if (Check3(something2))
    {
    double something3 = DoSomeWork3(something2);

    if (!Check4(something3))
    result = ERROR_4;
    }
    else
    {
    result = ERROR_3;
    }
    }
    else
    {
    result = ERROR_2;
    }
    }
    else
    {
    result = ERROR_1;
    }

    return result;
    }



    Here's the code if you check a flag instead:



    public int DemoMethod2(string param)
    {
    int result = ERROR_NONE;

    if (!Check1(param))
    result = ERROR_1;

    if (result == ERROR_NONE)
    {
    string something1 = DoSomeWork1(param);

    if (!Check2(something1))
    result = ERROR_2;
    }

    if (result == ERROR_NONE)
    {
    int something2 = DoSomeWork2(something1);

    if (!Check3(something2))
    result = ERROR_3;
    }

    if (result == ERROR_NONE)
    {
    double something3 = DoSomeWork3(something2);

    if (!Check4(something3))
    result = ERROR_4;
    }

    return result;
    }



    And this is what it looks like if you use multiple returns:



    public int DemoMethod3(string param)
    {
    if (!Check1(param))
    return ERROR_1;

    string something1 = DoSomeWork1(param);

    if (!Check2(something1))
    return ERROR_2;

    int something2 = DoSomeWork2(something1);

    if (!Check3(something2))
    return ERROR_3;

    double something3 = DoSomeWork3(something2);

    if (!Check4(something3))
    return ERROR_4;

    return ERROR_NONE;
    }



    "Aha!", you cry; "you should be using exceptions for error handling!"

    Indeed. But throwing an exception returns early from a method. It has a similar effect to a "return" statement, in that the normal flow of execution is curtailed, so if you say that there should only be one return statement, you should also say that there should be only one "throw" - or at the very least, all the throws should be in one place.

    Here's another example. This time, it's a method for searching for a char inside a string.

    Firstly, here's the code that sticks to the "only one return statement" dogma, as well as the "loop must only have one exit" dogma:



    public bool StringContains3(string text, char target)
    {
    bool found = false;

    for (int i = 0; !found && (i<text.Length); ++i )
    {
    if (text[ i ] == target)
    {
    found = true;
    }
    }

    return found;
    }



    If you're willing to allow a loop to have more than one exit, you can write it like this:



    public bool StringContains2(string text, char target)
    {
    bool found = false;

    foreach (char c in text)
    {
    if (c == target) // Loop has more than one exit!
    {
    found = true;
    break;
    }
    }

    return found;
    }



    And if you drop the dogma:



    public bool StringContains1(string text, char target)
    {
    foreach (char c in text)
    {
    if (c == target)
    {
    return true;
    }
    }

    return false;
    }



    Personally, I prefer to drop the braces for such simple code as that:



    public bool StringContains1(string text, char target)
    {
    foreach (char c in text)
    if (c == target)
    return true;

    return false;
    }



    Now, it seems to me that by allowing early returns in these cases, the code is clearer, not more opaque!


  • Larry Menard

    Why should it matter Even if they aren't identical in IL, the performance difference would be negligible.

    If you are explicit with what you intend your code to do the compiler can optimized better. Don't try and do the compiler's job; it can do it much better than you.

    This is a clear case of premature optimization.



  • ja123ee

    If you want to follow structured programming methodologies then you'd follow the "one entry one exit rule" and the code would be written:


    int Method(int cond, int a, int b)
    {
       int result = b; // default case
       if(cond)
       {
          result = a;
       }
       return result;
    }

     

    If you don't want to follow structured programming methodologies, I would suggest being clear with your intentions by using the else to clearly show that the second return is intended to be dependant on the test of cond:


    if(cond)
    {
       return a;
    }
    else
    {
       return b;
    }

     



  • Jonot2

    After have I read the posts , I think "One enter poit , One exit point" is a good principle.

  • tawammar

    it is same here, and also I don't think there is any difference in the runtime. Maybe I will perfer the first one, because it is easier to understand.

  • ShaneShowers

    I don't think there is any difference between these two code.

    As compiler generates almost same IL for this and so there is no difference at runtime.

    and as cgraus suggests, we should return only once in our code. it increases readability (not performance).



  • joeller

    Thanks, that's makes sense. So the code-looking part is solved.

    Now, when these examples

    if (cond) return a;
    return b;

    if (cond) return a;
    else return b;

    are executed exactly the same way in all cases (I hope they are :)), does the compiler make any optimizations which will result in having them same code in IL


  • desny

    The most common standard here is to define a default return value ( usually for failure ) at the top of your function, and set that value on the way through, having only one exit point at the end of your function.

    int retval = b;

    ...

    if (cond) retval = a;

    return retval;



  • else or not else?