Early returns vs nested positive if statements

Here is some hypothetical code sample:

if (e.KeyCode == Keys.Enter)
{
    if (this.CurrentElement == null) {
        return false;}

    if (this.CurrentElement == this.MasterElement) {
        return false;}

    if (!Validator.Exist (this.CurrentElement)) {
        return false;}

    if (!Identifier.IsPictureElement (this.CurrentElement)) {
        return false;}

    this.FlattenObjects(this.CurrentElement);
}

VS

if (e.KeyCode == Keys.Enter)
{
    if (this.CurrentElement != null) {

        if (this.CurrentElement != this.MasterElement) {

            if (Validator.Exist (this.CurrentElement)) {

                if (Identifier.IsPictureElement (this.CurrentElement)) {

                    this.FlattenObjects(this.CurrentElement);}}}}}}

}

Which one do you think is 开发者_如何转开发better in terms of readability, maintenance, etc?

Also the 2nd example can be formatted differently via the different use of parenthesis.

Early returns are much more readable.

Whenever you get more than four or five levels of nesting inside a method, it's time to refactor that method.

A single if with an || clause can sometimes be more readable:

if (this.CurrentElement == null
 || this.CurrentElement == this.MasterElement
 || !Validator.Exist(this.CurrentElement)
 || !Identifier.IsPictureElement(this.CurrentElement))
    return false;

The first example is better in every way. It's simpler, and easier to read. Some people say that every function should have a single return point; this example shows clearly why those people are wrong.

PS Personally I would remove all those superfluous curly brackets:

if (this.CurrentElement == null) return false;

etc. This makes it even simpler, and even easier to read.

I think I would write it like:

if (this.CurrentElement == null OR this.CurrentElement == this.MasterElement OR ...) return false;

I'd say the first is better for readability and maintenance. However, I'd probably write it something like this.

if (e.KeyCode == Keys.Enter) {
    if(this.CurrentElement == null ||
       this.CurrentElement == this.MasterElement ||
       !validator.exists(this.CurrentElement) ||
       !identifier.isPictureElement(this.CurrentElement))
    {
        return false;
    {
    else
    {
        this.flattenObjects(this.CurrentElement);
    }
}

Given that in the second example "false" is the return for all paths but it is implicit rather than declared why not just make all returns implicitly false and simply test for the one condition that is unique?

This might violate someone's style guidelines but logically is the most succinct.

if( e.KeyCode == Keys.Enter 
 && this.CurrentElement != null 
 && this.CurrentElement != this.MasterElement  
 && Validator.Exist (this.CurrentElement)                     
 && Identifier.IsPictureElement (this.CurrentElement)) 
    this.FlattenObjects(this.CurrentElement);