Coding style... error condition first or last?

Which is better in general in terms of the ordering? Do you put the fault condition at the top or bottom?

if (noProblems == true) {
    // do stuff
} else {
    // deal with problem
}

OR

if (noProblems == false) {
    // deal with problem
} else {
    // do stuff
}

From stackoverflow

• i like to eliminate error cases first - and return from the function early so that the 'happy path' remains un-nested, e.g.

  if (some error condition)
  {
      //handle it
      return;
  }
  //implicit else for happy path
  ...
  

  if it is easy to identify the conditions leading to the happy path, then by all means put that clause first (thanks Marcin!)

  Patrick McElhaney : Good answer. This pattern is commonly known as a "guard clause."

  Marcin : Sorry, I can't agree with this at all. Please see my answer below.

  John Rudy : +1 for guard clauses.

  Rob Prouse : +1 for happy path :D

  Steven A. Lowe : @DLarsen: answer edited to reflect Marcin's advice also

• if its 1 or 2 lines and an early return, I'd put it at the top -- especially if its at the start of the function. that almost reads like a contract. otherwise, I go with the "positive conditions before negative" rule.

• It depends on what is clearer to you. I mean, what makes more sense, that noProblems has a true value or that it has a false value.

  For example for isDeviceEnabled() I would always check for a true result as "Enabled" has implicitly a positive value. For an implicit negative value we could check for example for "isMemoryAvailable()", maybe because you need to check only if there is no more room for new objects/arrays or other data.

• It's a very individual sort of decision, but I tend to put my error conditions first, under the pretense of grouping like code together. That is, if I do something, and it fails, the check for that failure is really actually part of doing that something; if I put the check for failure and action based on that at the beginning of my code, I've grouped my action and the complex response to that action together (working on the assumption that a failure result is actually a more complex return case than a success).

• The first seems slightly preferable to me in that it avoids a double negative in the conditional.

  Other than that, you've constructed things so that you can't really

  // do stuff
  

  after you

  // deal with problem
  

  so beyond that one seems as good as the other.

• As with try/catch I do the normal flow, and then handle exception conditions.

  That doesn't mean error checking/scrubbing doesn't happen first, but that's if I don't trust what's been passed to me.

  e.g.,

  if (foo is null) then
      // bail
  end if
  
  if (bar == foo) then
      // foo hasn't been changed... print the regular report
  else
      // foo has changed, run the reconciliation report instead.
  end if
  

  I like the happy story to flow like a straight line, and the unhappy story to spin off onto its own spur.

• Maybe this depends on language conventions, or other factors, but I feel that the nominal case should be at the top, and branches should contain the exceptional conditions. It makes the code much easier to read. This is especially true when there are many exceptional conditions, and in most cases there are. You'll be able to easily assume that the author expects this specific path to be taken most of the time, and understand the code easier this way.

  From "Code complete, 2nd edition" section 15.1:

  By putting the most common cases first, you minimize the amount of exception-case handling code someone has to read to find the usual cases. You improve efficiency because you minimize the number of tests the code does to find the most common cases.

  Patrick Huizinga : You might also want to read section 17.1, which says: "Use guard clauses (early returns or exits) to simplify complex error processing"

  Steven A. Lowe : i would agree in the case when it is easy to identify the conditions that lead to the happy path, will edit my answer to reflect your additional wisdom

• Why not create one exit point from function and do proper cleanup there instead of having multiple returns...

  
  DWORD bRetVal= TRUE;
  if(foo is null) 
  {
     bRetVal = FALSE;
     goto Error;
  }
  else
  {
  // do something
  }
  
  Exit:
   return bRetVal;