3

In JavaScript I recently realized you could use the OR || logical operator for assignment, and I want to know if it's considered bad practice.

In particular I have some functions that have optional array input, if the input is null or undefined I should just set it to an empty array [], if it has content it should take the content.

I found that using the assignment using the OR operator handles that perfectly in a single line, it's clean. However, it feels like the kind of thing that might be considered bad practice, or may have some horrible pitfalls I'm not considering.

Another approach is a simple if check, which is fairly safe in general.

I want to know if using the || approach seen below has any pitfalls I'm not considering, although it works in this scenario I would appreciate knowing if it works well to keep using this in the future, or to stop using it altogether.

https://jsbin.com/nozuxiwawa/1/edit?js,console

var myArray = ['Some', 'Strings', 'Whatever'];

// Just assign using OR
var pathOne = function(maybeAnArray) {
    var array = maybeAnArray || [];

    console.log(array);
}

// Assign using IF
var pathTwo = function(maybeAnArray) {
    var array = [];

    // Covers null and undefined
    if (maybeAnArray != null) {
        array = maybeAnArray;
    }

    console.log(array);
}

console.log('Path one:');

pathOne(myArray); // ['Some', 'Strings', 'Whatever']
pathOne(null);    // []

console.log('\nPath two:');

pathTwo(myArray); // ['Some', 'Strings', 'Whatever']
pathTwo(null);    // []
Erick
  • 2,488
  • 6
  • 29
  • 43

4 Answers4

2

IMHO the use of the OR || for the purposes of assignment is perfectly valid and is good practice. We certainly use it in our projects and I've seen it used in lots of 3rd party projects that we use.

The thing you need to be aware of is how certain JavaScript objects can be coerced to be other values. So for example, if you're ORing values such as "", false or 0 then they are treated as false... this means that when you have the following:

function f(o) {
   var x = o || -1;
   return x;
}

Calling:

f(0) 

...will return -1... but calling

f(1)

Will return 1 ... even though in both cases you passed a number - because 0 is treated as false -1 is assigned to x.

...that said, as long as you're aware of how the OR operator will treat the operands that you use with it - then it is good JavaScript practice to use it.

Dave Draper
  • 1,837
  • 11
  • 25
  • I am in your camp. Just wanted to note that the problem with coercion is not specific to ||. You would still need to know and consider the issues with coercion when using an if statement. – j-a Apr 06 '16 at 07:18
  • Thanks. I was trying to keep the answer as simple as possible - I know it only really scratches the surface, but just wanted to try and call out the most obvious pitfall that I could think of! :) – Dave Draper Apr 06 '16 at 07:20
1

i prefer the first option, it's clear for my eyes, but when i need to share my code with others will think about to use second, will be more clear for any. Now i'm using sonar, and prefer the second option too, will more easy to comprend for machine in inegration works. Last idea is to use

if(maybeAnArray !== void(0))

Two reasons:

  1. use cast and type conditionals

  2. void(0) will works same for all browsers

Expect it helps yopu

Álvaro Touzón
  • 1,247
  • 1
  • 8
  • 21
1

When given the option, I prefer concise code (which must still be readable).

I would say || is common enough that it is considered good practice. Once one has seen it a few times it reads just fine.

j-a
  • 1,780
  • 1
  • 21
  • 19
1

In my opinion there are few reasons why you should rather use the second option:

  1. First of all it's much more readable - new developers that are still learning can have problems with understanding notation like var myArray = someArrayArg || [];
  2. If you are using some kind of code checkers like JSLint, they will return warnings and/or errors like Expected a conditional expression and instead saw an assignment. for the statement with var myArray = someArrayArg || [];
  3. We already have something like var myArray = someArrayArg ? someArrayArg : []; that works pretty well
Kamil
  • 2,712
  • 32
  • 39
  • 1. This is a common argument, I don't like that reasoning. With that logic, a new developer would not learn things they don't already know. Worse, a team would be defined by its lowest common denominator. – j-a Apr 06 '16 at 07:27
  • 2. Which version/options are you using? I am not getting any such warnings. – j-a Apr 06 '16 at 07:27
  • 3. I find this odd. You actually have both options || and the ternary operator. Why would you prefer a ternary operator taking three arguments when you only need an operator taking two arguments? – j-a Apr 06 '16 at 07:28
  • And on 2, just perhaps to nail the point. Even the source code for eslint itself is using || https://github.com/nodejs/node/blob/master/tools/eslint/lib/token-store.js – j-a Apr 06 '16 at 07:42