5
\$\begingroup\$

Is there a simpler/shorter/better way to do this?

let obj={a:1,b:[1,2]}

function add(key,value){
  if(!obj[key].push){
    obj[key]=[obj[key]]
  }
  obj[key].push(value)
}
\$\endgroup\$
10
\$\begingroup\$

Basically after reading over the code, I interpret that the conditional (i.e. !obj[key].push) checks if the value at the given key is not an array. A more robust way to do that is to use (the negated value of) Array.isArray() instead. That may not be any shorter, but perhaps a better way to determine if the property at key is an array.

var obj={a:1,b:[1,2]}

function add(key,value){
  if(!Array.isArray(obj[key])){
    obj[key]=[obj[key]]
  }
  obj[key].push(value)
}
add('a',3);
console.log(obj);

It would be difficult to prevent the re-assignment of the property .push (see example below). .push could be assigned to something other than a function, like an integer, string or object, or a function that does something other than push the supplied argument on the array.

var obj={a:1,b:[1,2]}

function add(key,value){
  if(!obj[key].push){
    obj[key]=[obj[key]]
  }
  obj[key].push(value)
}
obj.b.push = undefined;
add('b',3);
console.log(obj);

Edit

insertusernamehere made a good point in a comment: Perhaps it would be wise to guard against the case where the obj[key] is undefined. The current code would add that to the array, which is likely not preferable.

There are multiple ways to achieve this, including calling obj.hasOwnProperty(), checking the array returned by Object.keys(obj) does include key, etc.

var obj={a:1,b:[1,2]}

function add(key,value){
  if (!Object.hasOwnProperty(key)) {
      obj[key]=[];
  }
  if(!Array.isArray(obj[key])){
    obj[key]=[obj[key]]
  }
  obj[key].push(value)
}
add('a',3);
add('c',4);
console.log(obj);

\$\endgroup\$
  • 5
    \$\begingroup\$ I like your answer. I don't think that one should bother with possible reassignment of push, since a) it's an absolutely cornerstone method on arrays and messing with it will lead to deadly daemons; b) the add(...) function depends on that heavily (a spread operator could be a work around instead of a push for inserts, but it's potentially much costlier, especially on large arrays). The only "real" question here is "how to reliably identify an array in JavaScript", IMO. \$\endgroup\$ – Igor Soloydenko Oct 3 '17 at 20:37
  • \$\begingroup\$ @IgorSoloydenko so is what your saying that unless you use spread syntax this is probably the best way of doing it? And if you do use spread syntax their would be a definite performance penalty? \$\endgroup\$ – qw3n Oct 3 '17 at 20:57
  • \$\begingroup\$ Yes, you read me correctly. [...someArray, newValue] will iterate over someArray's values to produce a new array. So it's by definition somewhat costlier. This may or may not be a big deal for you. Btw, I'm can't guarantee that ... is not using .push() under the hood... And I can't see any problems in what you and Sam are having in your code snippets. \$\endgroup\$ – Igor Soloydenko Oct 3 '17 at 21:01
  • 2
    \$\begingroup\$ Maybe worth mentioning: The OP's and this solution don't test whether the key exists at all. If it doesn't, it will add an undefined value to the newly created array, for example when calling add('x', 1). \$\endgroup\$ – insertusernamehere Oct 3 '17 at 23:03
  • 1
    \$\begingroup\$ yeah - I guess I have an aversion to it now because of a linter used on a project at work that complains whenever it finds calls to .hasOwnProperty() but that is simpler. \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Oct 3 '17 at 23:50
3
\$\begingroup\$

Another possible way is to use concat instead of push. You may still want to do a hasOwnProperty check to ensure the property exists.

function add(key,value) {
  if (!obj.hasOwnProperty(key)) {
    obj[key]=[];
  }

  obj[key] = [].concat(obj[key], value);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.