Control Flow via Return vs. If/Else [closed]

Question

Which one is better (implicit control flow via return or control flow via if) -- see below. Please explain what you see as advantage/disadvantage to either one. I like option A because it's less code.

Flow via Return:

public ActionResult Edit(MyClass class)
{
    if (!class.Editable)
       return null;

    class.Update();
    return View();
}

Flow via If/Else:

public ActionResult Edit(MyClass class)
{
    if (class.Editable)
    {
       class.Update();
       return View();
    }
    else
    {
       return null;
    }
}

Answer 1

I prefer the second approach for the sake of readability and maintainability. Readability because it just reads 'cleaner' to me than the first approach, and maintainability because I don't have to worry about adding curly braces if I need to modify the if or else clauses. Further, the first approach is only 7 characters less than the second approach if you don't include new lines, which hardly seems a justification for choosing the first over the second.

That said, I actually prefer this:

public ActionResult Edit(MyClass class)
{
    ActionResult rv = null;
    if (class.Editable)
    {
        class.Update();
        rv = View();
    }
    return rv;
}

It's more code, but I can now set a single breakpoint on the return statement to inspect the value being returned instead of having to set two breakpoints to do the same in the two choices you offered.

Answer 2

I prefer the first option, provided the case you check is a guard/precondition that needs to be met for the method call to be valid. Although you could argue if you should return null, or throw an (Argument)Exception. When a class isn't editable, should it really be a parameter for this method?

Maybe a better option would be to create an IEditable interface and implementing this on the class you're passing an instance of right now.

Answer 3

I would prefer the one I identify as being the one which EXECUTES less code.
If it is more common to class.Editable being false then I'd go for A.

But this example does not give much of an advantage in either case.

In any given situation a developer should analyze the input and adjust the code to be optimized on the most common input data.

EDIT:
To clarify:
By executes less code I in reality mean is most efficient...

Answer 4

They are both valid options, and one isn't necessarily any better than the other. Which one you choose is, ultimately, personal preference. Yes, Option A results in slightly less code, but overall they are pretty much equal.

In both cases you are controlling flow via an if and a return. It's really a question of how you prefer to see your boolean logic - negative or positive?

Is ActionResult an enum or a base class? If it's an enum, why are you returning null when Edit returns what appears to be an enum? Wouldn't it be cleaner simply to return an ActionResult value that indicates no action was taken because the object wasn't in an editable state?

Answer 5

under these circumstances, I would go with option A. In this case you are doing your input validation and then preventing execution of the rest of the code if the input is not valid (not editable). This keeps the entire body of the function out of a big if/else statement and makes it more readable.

However, I would also consider raising an exception rather than retuning a null - that is assuming that passing in a non-editable object into an "edit" function isn't a normal occurrence.

Answer 6

I also prefer option 1. For me, it reads better like a book. Also I'm always pained by there not being a return at the end of option 2.