0
\$\begingroup\$

If I want to "push" an array into my object inside of a property, I must first assure that this property exists - it works the same way for arrays too. However, what is the smoothest way of doing this?

Currently, I am first assigning the property it's first initial value so it gets made: then, since the property exists, I can push data into it.

const result = {};
const data = [{
    name: "John Doe"
},
{
    name: "Doe John"
}];

const key = "helloworld";

if(result.hasOwnProperty(key)) {
  result[key].push(data);
} else {
  Object.assign(result, {
    [key]: data
  });
}

console.log(result);

Which gives me the following object.

{
    helloworld: [
        {
            name: "John Doe"
        }, 
        {
            name: "Doe John"
        }
    ]
}

What is the smoothest/better way to do this, if there is such a way?

\$\endgroup\$
2
  • \$\begingroup\$ If the property already exists on the outer object, would you want the new array items to be alongside the already existing array items? Eg { name: "John Doe" }, { name: "Doe John" }, { name: 'something else' }? \$\endgroup\$ Dec 23 '20 at 19:36
  • \$\begingroup\$ @CertainPerformance Yes, that would be the idea. \$\endgroup\$
    – P. Nick
    Dec 23 '20 at 19:43
1
\$\begingroup\$

Code as function

You have given code as an example. But even so always write functions as it make you think about what it is doing, and avoids experiments becoming a rambling mess.

Polymorphism

Adding properties as needed is handy in javascript as it is particularly good at polymorphism. One of its best features. It can go a long way in reducing memory overheads, reducing code complexity by avoiding the need to define inheritance, and improve productivity as a coder.

This does come with additional problems that in the long run can make using this pattern counter productive.

For example each time you want to access the property using this pattern forces you to repeat the predicate "Does property exist?" which becomes annoying, adds unneeded complexity and and slows down the code.

For example if you want to iterate the object result.helloworld you will need to write

if (result.helloworld) {
    //...iterate
}

While if the object came with helloworld: [] there is no need to do the test.

Ambiguity of intent

The code is written in such a way as to make your intent impossible to know.

You have added a prototype chain test result.hasOwnProperty(key)) which indicates that you do not trust the object's state.

There is no reason you have to assume all objects are in an untrusted state.

Just test if (obj[key]) rather than the verbose if (obj.hasOwnProperty(key)) {

If your intent is that of modifying an untrusted object, requiring you use result.hasOwnProperty(key))

Then if the property exists you can not then assume trust. The property as an Array which without trust is unknown. Calling push may throw an error

If the object does not have the own property but rather it is further up the prototype chain adding the new property to the object will make access to an existing property difficult, forcing all code that used the existing property to check which property to use.

Rewrite

Assuming that the object is trusted.

As a function

function addToArray(obj, name, ...data) {
    (obj[name] ?? (obj[name] = [])).push(...data);
}

Or

const addToArray = (obj, name, ...data) => {
    (obj[name] ?? (obj[name] = [])).push(...data);
}

call with

const o = {};
const data = [{A;1},{B:2}];
addArrayToObj(o, "foo", ...data);
\$\endgroup\$
1
  • \$\begingroup\$ When it's possible I agree that it's good to have pre-defined properties so you don't get that issue - however, there are also cases where you're dealing with user input and you don't actually know what they're going to pass in their object; which should always be checked either way. Which brings us to the intent of the untrusted object, and I completely agree. I think the explanations you gave helped me to understand it all on a deeper level, so I don't do the same mistake in the future. \$\endgroup\$
    – P. Nick
    Dec 24 '20 at 17:07
1
\$\begingroup\$

The overall approach looks fine - check if the property exists on the object, and then either create the collection or append the data to the existing collection. While there are other methods than yours that achieve the same thing, none of them is clearly smoother or more semantically appropriate - the tools to do this more concisely just don't exist in vanilla JS yet (though there are libraries like Lodash and Underscore that implement the same sort of logic under the hood).

But, your current code has a bug: if the property already exists on the object, the data will be pushed to the existing array as a subarray, rather than pushing all elements of data to the existing array:

const result = {
  helloworld: [{
    alreadyExistingProp: 'alreadyExistingVal'
  }]
};
const data = [{
    name: "John Doe"
},
{
    name: "Doe John"
}];

const key = "helloworld";

if(result.hasOwnProperty(key)) {
  result[key].push(data);
} else {
  Object.assign(result, {
    [key]: data
  });
}

console.log(result);

You want a flat structure instead. Change

result[key].push(data);

to

result[key].push(...data);

A couple other pointers:

hasOwnProperty? Since it sounds like the values of the object will always be truthy, you can consider making a truthy test instead of using hasOwnProperty:

if (result[key]) {

Object.assign is unnecessary when setting just a single property This:

Object.assign(result, {
  [key]: data
});

can be

result[key] = data;

Mutation worries? Note that the approach used so far will mutate the result object and the data array. Mutation may cause unexpected behavior and bugs elsewhere if other parts of the script have a reference to the object that gets modified. If this is something to worry about, either clone the data before assigning it, or immutably update the result variable (like how state is updated in React).

For a short contrived example of how accidental mutation might result in a bug:

const result = {};
const addData = (data) => {
  const key = "helloworld";
  if(result.hasOwnProperty(key)) {
    result[key].push(...data);
  } else {
    result[key] = data;
  }
};

const dataFromUser = [{
    name: "John Doe"
},
{
    name: "Doe John"
}];
addData(dataFromUser);
addData([{ name: 'Default Name' }]);


// now the `result` object contains all the data
// but the dataFromUser now contains an extra object, which may be a mistake:
for (const item of dataFromUser) {
  console.log('Name from dataFromUser:', item.name);
}

This may well not be a problem for your situation, but variable contents changing unexpectedly due to mutation by code elsewhere is quite a common source of bugs in my experience.

Of course, if you understand well how references to objects work, and have no problem with (or like) the mutation that will occur, feel free to use it.

\$\endgroup\$
4
  • \$\begingroup\$ The "Mutation worries?" Would be valid if the OP had tagged the question with "functional". However in JavaScript changing an instance's state is an important powerful technique. Blanket advise against (vague cons), without the pros, of using such techniques is poor advice. \$\endgroup\$
    – Blindman67
    Dec 24 '20 at 0:40
  • \$\begingroup\$ I can't count the number of times I've seen a question on Stack Overflow with the problem stemming from the accidental mutation of an object that was expected to stay unchanged. I think it's a reasonable thing to warn to be careful about. \$\endgroup\$ Dec 24 '20 at 0:52
  • \$\begingroup\$ Hiding a technique behind "DO NOT USE" does nothing to change the status quo, nor should the status quo dictate the validity of a technique.Without an unbiased explanation of the pros and cons it is just indoctrination based on subjective rather than objective reasoning, I too deal with many SO posts that do not understand references, particularly in the prevalent attitudes of "Mutation is Evil" preventing people from learning by using (experience). \$\endgroup\$
    – Blindman67
    Dec 24 '20 at 1:23
  • \$\begingroup\$ I am aware of the issue with mutation, especially in React - which is why I tend to go for immutable objects. Your points are completely right and I agree with them, I could just use a truthy statement instead of hasOwnProperty method - it's much safer indeed and all this is something I will consider in the future. As for the question, while you did come with really solid points, I feel like the other answer had more detailed explanation on the reasoning behind it. \$\endgroup\$
    – P. Nick
    Dec 24 '20 at 17:04

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.