3
\$\begingroup\$

I have an array of objects with, let's say, two properties. One of those properties can have bad values, in the form on a -9999 value.

The aim is to calculate the average of each property, without the bad values.

I am looking to improve my code, I feel especially wrong about the two loops.

var array = [
{item1: 50, item2: 10},
{item1: 70, item2: -9999},
{item1: 60, item2: 5},
{item1: 40, item2: 7},
]

var avg1 = array.reduce(function(sum, a,i,ar) { sum += a.item1;  return i==ar.length-1?(ar.length==0?0:sum/ar.length):sum},0);

document.getElementById("avg1").innerText = avg1;

var badvalues = 0;

for (var i=0; i<array.length; i++){
if (array[i].item2 == -9999){badvalues++;}
}

var avg2 = array.reduce(function(sum, a,i,ar) {
sum += (a.item2 == -9999? 0: a.item2);
return i==ar.length-1?(ar.length==0?0:sum/(ar.length-badvalues)):sum},0);

document.getElementById("avg2").innerText = avg2;
<span id="avg1"></span><br>
<span id="avg2"></span>

\$\endgroup\$
4
\$\begingroup\$

There is already a JavaScript function to aggregate values in an array: array.reduce(). You can use it like this:

const invalidValueForItem2 = -9999;

let result = {
    item1: { sum: 0, count: 0},
    item2: { sum: 0, count: 0},
};

result = array.reduce(function(acc, item) {
    acc.item1.sum += item.Item1;
    ++acc.item1.count;

    if (item.Item2 != invalidValueForItem2) {
        acc.item2.sum += item.Item2;
        ++acc.item2.count;
    }
}, result);

Using an object to hold your temporary data we may scale better when you will need more properties and/or if you will average all properties without any knowledge about them. Don't you need to average both Item1 and Item2? Just drop the outer object and keep { sum: 0, count: 0}:

let result = { sum: 0, count: 0};

result = array.reduce(function(acc, item) {
    if (item.Item2 != invalidValueForItem2) {
        acc.sum += item.Item2;
        ++acc.count;
    }
}, result);

Now you have your result you can calculate the average:

const average2 = result.item2.count > 0 ? (result.item2.sum / result.item2.count) : NaN;

Do not forget to handle the corner case where there aren't items to sum, in this example when there aren't items the average is NaN (but you might want 0), if Infinity is appropriate then just make the division.


Few more notes about your code:

Indentation is incredibly important. Write just few hundred lines without proper indentation and you won't be able to understand what your code is doing without re-reading everything (and probably introducing bugs here and there).

Unless you have to support older browser you'd better use let instead of var (and const where appropriate).

Let your code breath, add space (both horizontally and vertically) and it will be much easier to read. To make code unreadable but small is the job of Minifiers/Uglifiers! Compare this with your original call to reduce():

var avg1 = array.reduce(function(sum, a, i, ar) {
    sum += a.item1; 
    return i == ar.length - 1 ? (ar.length == 0 ? 0 : sum / ar.length) : sum;
}, 0);

Those lines let me introduce few more things:

Do not nest ternary operators unless you're dealing with incredibly simple expressions.

You do not need all the parameters of the reduce() function (ar is just array) then it might be function(sum, a). Also you'd better find a meaningful name for a (especially in this case because function documentation talks about an accunulator then it may be misleading if you do not work often with it). Removing i will also stop you to do two things in this function (accumulate the sum and then return the average for the last one).

If your accumulator is a number (initialized to zero) then you do not need to explicitly give it as parameter.

How can you rewrite this? Just few lines when using an arrow function:

let sum = array.reduce((acc, item) => acc + item.item1);
let average = sum / array.length;

However you won't need this code because average for Item1 is calculated together with Item2, use this code for simpler cases.

\$\endgroup\$
2
\$\begingroup\$

I'm assuming from your question that the -9999 values can only appear in item2?

If that's the case then this works:

var array = [
{item1: 50, item2: 10},
{item1: 70, item2: -9999},
{item1: 60, item2: 5},
{item1: 40, item2: 7},
]

sum1 = 0;
avgcount1 = array.length;
sum2 = 0;
avgcount2 = 0;

for (i = 0; i < array.length; i++) {
    sum1 += array[i].item1;
    if (array[i].item2 != -9999) {
        sum2 += array[i].item2;
        avgcount2 += 1;
    }
}

avg1 = sum1 / avgcount1;
avg2 = sum2 / avgcount2;

document.getElementById("avg1").innerText = avg1;
document.getElementById("avg2").innerText = avg2;
<span id="avg1"></span><br>
<span id="avg2"></span>

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