Style of if: to nest or not to nest

Question

A colleague of mine and me had a discussion about the following best-practice issue.
Most functions/methods start with some parameter checking.

I advocate the following style, which avoids nesting.

if (parameter one is ugly) return ERROR;
if (parameter two is nonsense || it is raining) return ERROR;
// do the useful stuff
return result;

He, who comes from a more functional/logic programming background, prefers the following, because it reduces the number of exit points from the function.

if (parameter one is ok) {
   if (parameter two is ok && the sun is shining) {
      // do the useful stuff
      return result
   }
}
return ERROR;

Which one would you prefer and why?

Answer 1

The second one I'd prefer usually, unless the whole function body will be wrapped in the X number of if statements. If so, I'd choose the first option.

Answer 2

As long as the style is consistent across the codebase, any of these two styles would be ok with me.

Answer 3

I prefer the first. 5298529357 levels of indents just grates on my nerves.

Add to that the fact that when you return right away, it's obvious that (parameter one is ugly) is an error.

Answer 4

I prefer to do all my input parameter validation at the beginning of the function and do a return there only. Hence I prefer the first approach most of the times. If there is only one level of nesting then I might go for the second option.

Answer 5

I personally prefer the first style, since I feel that it provides some logical separation between what we can call "error cases" and "method logic". There is a well defined block in the beginning of the method that evaluates and acts on any errors in the input, and then the rest of the method is all about what the method should actually do.

It's some sort of separation of concerns on the micro level, I guess.

Answer 6

The most readable style is:

if (parameter one is ok) 
{
   if (parameter two is ok && the sun is shining) 
   {
      // do the useful stuff
      return result
   }
   else
   {
      // do other things
   }
}
return ERROR;

At least for me :)

EDIT: Sorry, misunderstood the question. I vote for the first, don't like deep nesting.

Answer 7

In the case of two checks then either is ok really, once you add more, option 1 quickly becomes more and more desirable!

Answer 8

In my opinion it only depends on the type of error checking you need.
If for example parameter one is ugly but in the following code you can manage to change it's state to pretty the second method is preferable.
However if the error is fatal and cannot be handled you should return immediately.
There is a third option here where the second style fits the most and it is when you want to gather all of the errors into one coherent error message.
The second style shouldn't check for validness but for non-validness first.
As for personal preference I'd be much happier with the first style.

Answer 9

Our in-house style is to avoid multiple return points and also to limit the amount of nesting, so I'd probably combine your pre-condition sanity checks together and do something like:

result_t result = OKAY;    

// Sanity checks
if ((parameter_one == ugly) || (parameter_two == nonsense) || (weather == raining))
{
    result = ERROR; 
}
else
{
    // do the useful stuff 
}

return result;