HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

If statement, is the shortened version readable enough?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thereadablestatementversionshortenedenough

Problem

I have to perform an IF statement in my Javascript code. I utilised the first method shown below:

(someVar1 !== "someString") && (someVar2[i].disabled = (someVar3.find("." + someVar4).length == 0));


Is the shortening of an if like this ok to do if my code is to be viewed by other members of my team? I have been told that it is a bit unclear and should instead use something like the following instead:

if (someVar1 !== "someString") {
  var bool = someVar3.find("." + someVar4).length == 0;
  someVar2[i].disabled = bool;
}


What are people's thoughts on this? Was it reasonable for me to implement the first method? Was it fair for me to be told I should change it and not use the (someVar) && doThis version of an if?

Solution

The second part of the condition in the first method (someVar2[i].disabled = (someVar3.find("." + someVar4).length == 0)) is only an assignment and is always evaluated to true.

It is clearer to use only the first part of the condition as a condition in if (someVar1 !== "someString") and move assignment inside the if statement, similarly as you did in the second method.

Regarding the second method, I would avoid using names such as bool as they are keywords. Use another name instead.

Also, in this particular case you are assigning value to variable 'bool' and then use it only once. You could assign the value directly to someVar2[i].disabled.

if (someVar1 !== "someString") {
  someVar2[i].disabled = someVar3.find("." + someVar4).length == 0;
}


You could also use inline if:

someVar2[i].disabled = (someVar1 !== "someString") ? (someVar3.find("." + someVar4).length == 0) : ;

Code Snippets

if (someVar1 !== "someString") {
  someVar2[i].disabled = someVar3.find("." + someVar4).length == 0;
}
someVar2[i].disabled = (someVar1 !== "someString") ? (someVar3.find("." + someVar4).length == 0) : <defaultValue>;

Context

StackExchange Code Review Q#23078, answer score: 6

Revisions (0)

No revisions yet.