Average calculation error

Question

I have this contract that calculate average

pragma solidity ^0.4.24;

contract test{

uint average;
uint counter;

function calcAverage( uint _newValue) public
{
    average = ((average * counter) + _newValue)/(counter + 1);
}

function get() public view returns(uint)
{
    return average;
}
}

I call to calcAverage function and I send 100, then i get the value and I recover 100. Fine. The problem is when I insert the new value. I insert 700 and the recalculate average is 700. Not the expected 400

Answer 1

A supplemental to the previous answer:

You might want to maintain the total sum instead of the average, for two reasons:

1. By maintaining the average, you have an accumulated precision-loss; If you maintain the total sum instead, then you will lose precision only when an external user requests the average.
2. The mining-fee (gas) of calculating the average every time a new value is sent, can be significantly higher than just calculating the total sum (while calculating the average when requested from the off-chain is free of course, since no mining is required).

In short, here is what you could do (optimizing the code in the previous answer):

pragma solidity ^0.5.1;

contract test {
    uint total;
    uint counter;

    function calcAverage( uint _newValue) public
    {
        total += _newValue;
        counter++;
    }

    function get() public view returns(uint)
    {
        return total / counter;
    }
}

Note that I have not referred to arithmetic overflows here (neither did you, and nor did the author of the previous answer). You may want to take care of that, by changing this:

total += _newValue;
counter++;

To this (after importing SafeMath library and using it in your contract):

total = total.add(_newValue);
counter = counter.add(1);

For the counter to overflow, one will need to call the function 2^256 times, which is approximately the number of atoms in the known universe. Technically speaking, there is not enough energy in the world to make that happen, so it's your call whether or not to apply this protection...

Answer 2

You forgot to increase your counter variable. So just add that.

Here's the code:

pragma solidity ^0.5.1;

contract test {
    uint average;
    uint counter;

    function calcAverage( uint _newValue) public
    {
        average = ((average * counter) + _newValue)/(counter + 1);
        counter++;
    }

    function get() public view returns(uint)
    {
        return average;
    }
}