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)
}
\$\begingroup\$
\$\endgroup\$
Add a comment
|
2 Answers
\$\begingroup\$
\$\endgroup\$
7
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);
-
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) theadd(...)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– Igor Soloydenko2017-10-03 20:37:51 +00:00Commented Oct 3, 2017 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– qw3n2017-10-03 20:57:08 +00:00Commented Oct 3, 2017 at 20:57
-
\$\begingroup\$ Yes, you read me correctly.
[...someArray, newValue]will iterate oversomeArray'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– Igor Soloydenko2017-10-03 21:01:17 +00:00Commented Oct 3, 2017 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
undefinedvalue to the newly created array, for example when callingadd('x', 1). \$\endgroup\$insertusernamehere– insertusernamehere2017-10-03 23:03:46 +00:00Commented Oct 3, 2017 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\$2017-10-03 23:50:01 +00:00Commented Oct 3, 2017 at 23:50
\$\begingroup\$
\$\endgroup\$
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);
}
