What is the accepted practise for returning from boolean methods

Question

Okay, this is a simple question, but I'd like some oppinions on the correct practice here. I am not looking at this for performance concerns, because CPU's are so powerful that this wouldn't make any perceivable difference unless called without a looping contruct with thousands of iterations. I just want views on what is the accepted standard.

I have a method that bascially just does a check returns a boolean. However, there are numerous ways to implement this.

Here is how I would normally implement this.

    public bool CanUndo()
    {
        if (_nCurrentUndoIndex > 0)
            return true;
        else
            return false;
    }

However, it is often frowned upon to return from the middle of a method. The only time I normally do this is when performing a check on a form submission like this.

        if (String.IsNullOrEmpty(firstName.Text))
        {
            MessageBox.Show("Please enter a first name", "Incomplete");
            return;
        }

I consider that acceptable.

Back to the undo question, an alternative way to code it would be this.

    public bool CanUndo()
    {
        bool returnVal;
        if (_nCurrentUndoIndex > 0)
            returnVal = true;
        else
            returnVal = false;
        return returnVal;
    }

This however unncessarily allocates a variable and is more verbose code. Another option would be.

    public bool CanUndo()
    {
        bool returnVal = false;
        if (_nCurrentUndoIndex > 0)
            returnVal = true;
        return returnVal;
    }

This is more streamlined as it gets rid of the else. However, if the value is true is makes an unneccesary assignment by initializing it to false.

you're worrying about a detail that doesn't matter much. The compiler will likely optimize away most of the differences, and even if it didn't, it's unlikely you would notice the difference unless you were talking loops of 100's of millions (not thousands). — hatchet - done with SOverflow, Aug 03 '11 at 13:52

score 2 · Accepted Answer · edited May 23 '17 at 12:04

2

public bool CanUndo () {
    return _nCurrentUndoIndex > 0;
}

Personally I don't have a problem with returning from the middle of a method. It complicates cleanup code for C functions but with RAII that argument disappears.

I prefer to exit as soon as is suitable otherwise you get

if (x) {
   if (y) {
       if (z) {
          complete
       }
   }
}

rather than

if (!x)
    return

if (!y)
    return

if (!z)
    return

complete

This way you avoid nesting, wide lines (horizontal screen space is expensive, vertical space is cheap) and you always know that if you're still in a function then you're not in an error path. Code which works well with this design also works well with exceptions, which is very important.

edited May 23 '17 at 12:04

Community

1
1

answered Aug 03 '11 at 13:47

spraff

32,570
22
121
229

Yes, I forgot that alternative, bascially what we would have implemented as an inline function, implemented in the header file, back in my C++ days. – WPFNewbie Aug 03 '11 at 13:52
Perhaps I picked too simple of a scenario. If the condition was more complex then a simply > comparision and it couldn't be implemented in a single line and maintain readability which of the three options would you choose. – WPFNewbie Aug 03 '11 at 13:53
Provided that it doesn't require adding cleanup code, I would take the multiple-returns route. If you have to duplicate the cleanup code for the extra branches then it becomes a matter of taste. For small functions it isn't important, for big ones, refactor them into smaller ones. – spraff Aug 03 '11 at 13:56

score 1 · Answer 2 · answered Aug 03 '11 at 13:55

you should always contract boolean returns to their logical aquivalent, because this is much easier to read for developers, it is faster to write for you and it get contracted by the compiler anyways.

consider an expanded or:

if (a == 1)
    return true;
else if (a == 2)
   return true;
else if (a == 3)
   return true;
else 
   return false;

and the reason should become obvious when you compare it to the contracted version

return (a == 1) || (a == 2) || (a == 3)

score 0 · Answer 3 · edited Aug 03 '11 at 13:52

0

public bool CanUndo()
{
   return (_nCurrentUndoIndex > 0);
}

edited Aug 03 '11 at 13:52

spraff

32,570
22
121
229

answered Aug 03 '11 at 13:50

What is the accepted practise for returning from boolean methods

3 Answers3