Correct JavaScript IF Else Syntax

Question

I am updating an existing JavaScript application. Part of this update involves formatting things better and cleaning up code.

Right now this is one of the Functions which uses if() and else statements without all the correct brackets in place... I personbally hate it when people do this shorthand method and even more so when they mix it and use it sometimes and not others. THat is the case in this example code below.

this.setFlags = function() {
    if (document.documentElement)
        this.dataCode = 3;
    else
    if (document.body && typeof document.body.scrollTop != 'undefined')
        this.dataCode = 2;
    else
    if (this.e && this.e.pageX != 'undefined')
        this.dataCode = 1;

    this.initialised = true;
}

I honestly do not know how to correctly add the brackets to this function, below is what I have but I am thinking it might not be correct. Can a JavaScript expert let me know?

this.setFlags = function() {
    if (document.documentElement){
        this.dataCode = 3;
    }else{
        if (document.body && typeof document.body.scrollTop != 'undefined'){
            this.dataCode = 2;
        }else{
            if (this.e && this.e.pageX != 'undefined'){
                this.dataCode = 1;
            }
        }
    }

    this.initialised = true;
}

It's correct. Congrats on adding the brackets correctly. Is that your only question? — Dave Chen
– Dave Chen, Commented Oct 5, 2014 at 4:44
Both are fine, when you have to write just one line in if statement, there is no need to put {}. — Mritunjay
– Mritunjay, Commented Oct 5, 2014 at 4:45
not using {} on single line if cases/conditions is often considered a bad practice, because errors can hide in plain sight if you have poor indentation. stackoverflow.com/questions/4797286/… — Allan Nienhuis
– Allan Nienhuis, Commented Oct 5, 2014 at 5:00

Ted Hopp · Accepted Answer · 2014-10-05 04:59:38Z

8

I share your dislike of unbracketed if/else statements. Your bracketing seems correct to me. However, there's a simpler way that's equivalent:

this.setFlags = function() {
    if (document.documentElement) {
        this.dataCode = 3;
    } else if (document.body && typeof document.body.scrollTop != 'undefined') {
        this.dataCode = 2;
    } else if (this.e && this.e.pageX != 'undefined') {
        this.dataCode = 1;
    }

    this.initialised = true;
}

As Dave Chen points out in a comment, because of the simplicity and parallelism of the if/else branches—they all assign a value to this.dataCode—you could also use nested ternary operators:

this.setFlags = function() {
    this.dataCode = document.documentElement                           ? 3
                  : document.body
                      && typeof document.body.scrollTop != 'undefined' ? 2
                  : this.e && this.e.pageX != 'undefined'              ? 1
                  :                                                      this.dataCode;

    this.initialised = true;
}

The advantage of this is that it is clearer that all the conditions are simply determining which value to assign to this.dataCode. The disadvantage (a big one, in my opinion) is that unless the nested ternary operators are formatted in a careful way (such as what I did here), it's basically impossible to get any sense of what's going on without carefully studying the expression. Unfortunately, most JavaScript code formatters that I'm familiar with do a very poor job of formatting such expressions (or preserving such formatting). (Interestingly, several Perl formatters that I've used do a wonderful job of this. But this isn't Perl.)

edited Oct 5, 2014 at 4:59

answered Oct 5, 2014 at 4:45

Ted Hopp

235k48 gold badges412 silver badges533 bronze badges

Sign up to request clarification or add additional context in comments.

9 Comments

Dave Chen Over a year ago

You could also use ternary, but I don't think that's what the OP is asking. The question isn't, "How can I make this code better?".

Ted Hopp Over a year ago

@DaveChen - Yes, that's another possibility. I'll add that to my answer. Thanks.

JasonDavis Over a year ago

It's great, I know it was a super basic question but the way it was formatted it wasn't all that clear to me, thanks and yes improvements are always welcome!

Ted Hopp Over a year ago

@DaveChen - Well, I take that back. There's no unconditional else case so there is no value to use as the last operand in a compound ternary expression (when all three if tests fail).

Dave Chen Over a year ago

No, because you could just set it to itself. Ternary is ugly sometimes -- but always works. ex. this.dataCode == whatever ? set value : this.dataCode

|

hola · Accepted Answer · 2014-10-05 04:49:08Z

1

It's correct but you can make it neater:

this.setFlags = function() {
  if (document.documentElement) {
    this.dataCode = 3;
  } else if (document.body && typeof document.body.scrollTop != 'undefined') {
    this.dataCode = 2;
  } else if (this.e && this.e.pageX != 'undefined') {
    this.dataCode = 1;
  }

 this.initialized = true;
};

oh this is already posted

answered Oct 5, 2014 at 4:49

hola

3,55017 silver badges24 bronze badges

Collectives™ on Stack Overflow

Correct JavaScript IF Else Syntax

2 Answers 2

9 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

9 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related