Javascript condition optimization

Question

Can the following condition be optimized/simplified ?

if (!this.properties.width || 
 this.properties.width <= 0 || 
 !this.properties.height || 
 this.properties.height <= 0)
 return;

So basically if properties does not have a width or height property or the width or height property is less than or equal to zero then return

• Vogel612: Signatures are discouraged in the complete stackexchange Network. For more information you might want to read this meta post
• Bergi: What values exactly do you expect here? undefined for non-existing properties, integers otherwise? Do width and height actually ever become negative?

Answer 1

I'd simply flip it around:

if( !(this.properties.width > 0 && this.properties.height > 0) ) {
 return;
}

This will catch null, undefined, NaN, etc., etc.. It will allow numeric strings (provided they can be coerced to a number greater than zero), and of course it'll allow straight-up positive, non-zero numbers.

---

Update: There's one edge-case I've come across: Arrays. It's some unfortunate JavaScript weirdness to do with type coercion:

[] > 0; // => false (so far so good)
[0] > 0; // => false (as it should be)
[1] > 0; // => true (what?)
[1, 1] > 0; // => false (WHAT?)

So an array with just a single numeric element will be treated as a number when comparing.

Well, actually, as far as I can tell, the array is coerced to a string by coercing each element to a string, and joining them with a comma, and the joined string is then (削除) compared lexicographically (削除ここまで) coerced to a number in order to be compared. (Thanks to Robert in the comments for the correction)

[1] + 0; // => "10"
[1, 1] + 0; // => "1,10"
"0" > 0; // => false
"1" > 0; // => true
"1,1" > 0; // => false

I love JavaScript, but sometimes... jeez...

• netpoetica: It's also worth noting that you can get a little performance boost by not performing two scope lookups on this.properties by caching var props = this.properties; and then just accessing as props.width and props.heightin your logic
• Robert: It doesn't compare lexicographically. Instead the resulting string gets coerced to a number. But since numbers don't contain commas +'1,1' is NaN.
• Flambino: @Robert Ah, right. Thanks for clearing that up; I'll note it in the answer

Answer 2

While !this.properties.width tests if the value is undefined (i.e. it does not exist) it does not tell someone who comes along to maintain it later that that was your intention.

You could write:

if (
 this.properties.width === undefined
 || this.properties.width <= 0
 || this.properties.height === undefined
 || this.properties.height <= 0
)
{
 return;
}

or if the intention is to exit the function early if width or height is not a number then you could use:

if (
 isNaN( this.properties.width )
 || this.properties.width <= 0
 || isNaN( this.properties.height )
 || this.properties.height <= 0
)
{
 return;
}

DRYNESS

If the same test is going to be repeated for many properties or repeatedly throughout the code then you could wrap it in an appropriately function:

function isPositiveNonZeroNumber( value )
{
 return !isNaN( value ) && value > 0;
}

then you can write:

if (
 !isPositiveNonZeroNumber( this.properties.width )
 || !isPositiveNonZeroNumber( this.properties.height )
)
{
 return;
}