3
\$\begingroup\$

I was given this task: Calculate amount of elements in collection by property: "name"

 let array = [
    {name: 'Serhii'},
    {name: 'Serhii'},
    {name: 'Nile'},
    {name: 'Apple'},
    {name: 'Serhii'},
    {name: 'Serhii'},
    {name: 'Apple'},
    {name: 'Serhii'},
];

Here is my solution:

 function calculateElements(property, arr) {
    let amount = {};

    arr.forEach((e) => {
        let p = e[property];
        p in amount ? amount[p]++ : amount[p] = 1;
    });
    return amount;
}
console.log(calculateElements('name', array));

Is there more efficient way to do it?

\$\endgroup\$
1
  • \$\begingroup\$ not bad but i'd rewrite with reduce instead of forEach \$\endgroup\$
    – Jonah
    Oct 12, 2016 at 16:29

2 Answers 2

2
\$\begingroup\$
  • The function name should probably not be calculateElements() but rather something like countDistinctValuesByProperty() which has more specific meaning.
  • I would suggest that what you are doing here is, in essence, a map-reduce problem and perhaps should leverage existing Array functionality for this.

For example:

function countDistinctValuesByProperty(prop, array) {
    return array.map(el => el[prop]);
                .reduce(
                    (counter, value) => {
                        return value in counter ?
                            counter[value]++ : counter[value] = 1;
                    }
                    , {});
}

Or in this simple case, we could probably just go straight to reduce:

function countDistinctValuesByProperty(prop, array) {
    return array.reduce(
       (counter, el) => {
           if (prop in el) {
               return el[prop] in counter ?
                   counter[el[prop]]++ : counter[el[prop]] = 1;
           }
           return counter;
       , {});
}
  • This second solution is more optimal for this particular need, as it would operate in O(n) as opposed to O(2n) for the first. I just show the first solution to prompt your thinking on how you might approach a scenario where you needed to perform more complex mapping logic on input array before being able to get to the reduce step.
  • You do nothing to validate the parametric input on your function. I would think you should validate the property name parameter as being non-zero-length string. You also might consider validating that array is in-fact an Array object before trying to work with it. In either case if validation fails, you can exit early.
\$\endgroup\$
2
\$\begingroup\$

That looks quite fine to me. You can remove the parentheses around e in the arrow function.

If you're really going for efficiency, a regular for loop is faster than the .forEach() method. That can help if you have a very large amount of objects to process and need to display a result instantly to a user, but otherwise the optimization is unnecessary for what you're trying to accomplish.

\$\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.