a single equals in an if. Javascript. Any good reason?

Question

jQuery.each(player, function(key, val){                     
     if (el = $("#pr_attr_plain_"+key)){
          el.text(val === "" ? 0 : " " + val);
     }
});

I inherited a project and I came across something strange. The guy who started the project is an expereinced programmer, definitely more so then myself. Is there any value or reason (no matter how bad) in doing this:

if (el = $("#pr_attr_plain_"+key))

It works now and it's in a portion of the code that I don't have to touch. I don't want to change it and have it produce unexpected consequences without knowing what it might do.

Answer 1

Uh, maybe testing the $ operator? I would hazard a guess that it will return false if there is an error in the assignment.

Answer 2

If the el = $("#pr_attr_plain_"+key) evaluates to 0 or false or null or undefined, it won't go inside the if-block. In all other cases it will.

Answer 3

It can be correct. The code is equivalent to:

jQuery.each(player, function(key, val){                     
     el = $("#pr_attr_plain_"+key);
     if (el){
          el.text(val === "" ? 0 : " " + val);
     }
});

Answer 4

Right now this sets el to $("#pr_attr_plain_"+key). This tests the assignment. This would return true unless $() fails, returning null, false or 0.

Answer 5

It's just an ugly way of writing

 el = $("#pr_attr_plain_"+key);
 if (el){
      el.text(val === "" ? 0 : " " + val);
 }

which is in turn the same as

 if ($("#pr_attr_plain_"+key)){
      $("#pr_attr_plain_"+key).text(val === "" ? 0 : " " + val);
 }

but slightly faster (one less call to the $ function) and arguably easier to read. On the other hand, there are several problems with it:

• $(something) will never evaluate to false (it returns a jQuery object, which is truthy). Whoever wrote this probably meant $(something).length.
• there is no need for the whole if. You can just write $("#pr_attr_plain_"+key).text(val === "" ? 0 : " " + val); and it will have no effect if the jQuery selector does not match any element.
• unless el has been used previously, you should write var el =... not el = otherwise el will be global.
• since it is a common mistake to accidentally write = instead of ==, it is advisable to either avoid assignments inside conditions or use an extra parenthesis to make it clear you were doing it deliberately:

.

 if ((el = $("#pr_attr_plain_"+key))){
      el.text(val === "" ? 0 : " " + val);
 }

Answer 6

Assignment in an if statement isn't that common, but it does crop up a in some languages that support it, and is often used to make code more concise.

It's usually reserved for situations where you want to assign a value to an interim variable to both test it for 'truth' and then do something else with it. So, for example:

if (foo = performExpensiveOperation()){
    doSomethingElse(foo); // this only happens if 'foo' is true
}

..is a teeny bit shorter than the alternative:

foo = performExpensiveOperation();
if (foo) {
    doSomethingElse(foo);
}

Unfortunately, Javascript doesn't allow the var keyword in an if statement (ie, if (var foo = bar) is a syntax error), which means you have to declare a variable up front anyway (or, worse, use a global variable).

I see it used in Perl a fair bit, too, as it also has the effect of limiting the variable to the if's lexical scope. Which, again, is something Javascript can't do either (well, support for let is getting there).

Answer 7

As pointed out on JSLint, it is a bad practice since it can also be a common mistake. Better to fix it and reduce the chances of the wrong interpretation.