Print the average of std::vector

Question

As title suggests I'm trying to learn how to print out the average of a vector. Which can be done with:

int avgVec(const vector<int> & x)
{
   int sum=0;
   for(int i=0; i<x.size(); ++i)
    {
    sum+=x[i];//sets sum to vector size

    }
   return sum/x.size(); //Now calculates the average
}

Has this been set-up correctly?

Now the vector in question takes input from the user which should be set up as:

vector<int> myVec;
int i;
do
{
cin >> i;
myVec.push_back(i);
}while(i>0);

Has this also been set-up correctly?

Now in order for this to print the average would I go about making a print function or simply after doing:

avgVec(myVec); 

I'd print out myVec, or have I gone about this in the wrong way? I've executed the code and it does in fact compute the average, but it only does the first number inputed, which makes the average 1. I feel like I'm right there, but one part is just not clicking.

Answer 1

This code snippet is wrong

vector<int> myVec;
int i;
do
{
cin >> i;
myVec.push_back(i);
}while(i>0);

because you will enter one non-positive number that is the last number of the vector will be non-positive.

The valid code would look as

vector<int> myVec;
int i;
while ( cin >> i && i > 0 ) myVec.push_back(i);

The function also is wrong because nothing prevents the vector to be empty. So I would write it the following way

int avgVec( const vector<int> & v )
{
   int sum = 0;

   for ( int x : v ) sum += x;

   return v.empty() ? 0 : sum / v.size();
}

Answer 2

use the C++ standard library:

double avgVec(std::vector<int> const&v) // note: the average must not be an integer
{
   return v.empty()? 0.0 :
          std::accumulate(v.begin(), v.end(), 0.0)) / v.size();
}

Answer 3

The main problem with your code is that you're using integer arithmetic.

E.g. with integer arithmetic 7/3 → 2, with 1 as remainder.

Instead do e.g. 1.0*std::accumulate( v.begin(), v.end(), 0 )/v.length(), where the multiplication by 1.0 forces conversion to double (the default floating point type in C and C++), and return a double.

---

An alternative to multiplying by 1.0 is to use an explicit static_cast, which is more general but also more verbose.

Another alternative, less general but more elegant for this case, is to use 0.0 instead of integer 0 as the starting value for std::accumulate, as noted by KitsuneYMG in a comment. This works because the starting value determines the value type for the accumulation and its result.

---

Also, note that your current input code pushes the last non-positive input value on the stack, so if the user enters 0 as the last value, it will contribute to the count but not to the sum, thus reducing the average a bit…

Answer 4

Instead of writing your own loop for summing up the vector, you should use std::accumulate (like Walter suggested), but - don't pass a vector reference, even if it's just a const.

Let's take inspiration from std::accumulate's interface:

template <class InputIt, class T>
T accumulate(InputIt first, InputIt last, T init);

And write something like:

template <typename InputIt, typename Result = double>
Result average(InputIt start, InputIt end)
{
    static_assert(
        std::is_arithmetic<std::decay_t<decltype(*start)>>::value,
        "Can only average arithmetic values");
    return static_cast<Result>(
        std::accumulate(start, end, static_cast<Result>(0)) / 
        std::distance(start, end)
    );
}

Notes:

• I don't like first and last, since last is not really the last, it's one past the last element; so I use start and end instead.
• If you were using C++20, you could constraint the templates themselves; and you could also consider std::reduce instead of std::accumulate.

And if you want a function to call for a container, write that too:

template <typename Container, typename Result = double>
Result average(const Container& c)
{
   using const_iter = typename Container::const_iterator;
   return average<const_iter, Result>(std::cbegin(c), std::cend(c));
}

... and if you're in C++20, you could instead write this for a range rather than an arbitrary type.

See this running on GodBolt.

Answer 5

Looks like you're using zero to stop your input loop but still including it in the vector. This might be your intent, but it does affect the result.

Answer 6

Idiomatic C++ is:

int avgVec(const std::vector<int> & x)
{
   int sum = 0;

   for(std::vector<int>::iterator it = x.begin(); it != x.end(); ++it)
      sum += *it;

   return sum / x.size();
}