Deviating from the norm

Back on track with my C++ saga:


The standard deviation of a list of numbers is a measure of how much
the numbers deviate from the average. If the standard deviation is
small, the numbers are clustered close to the average. If the
standard deviation is large, the numbers are scattered far from the
average. The standard deviation, \$ S \$, of a list of \$ N \$
numbers \$ x \$ is defined as follows


$$ S = \sqrt{\dfrac{\sum\limits_{i=1}^{N} \left( x_i - \bar{x} \right)^{2}}{N}} $$


Where \$\bar{x} \$ is the average of the \$ N \$ numbers \$ x1, x2, .... \$ Define a function that takes a partially filled array of
numbers as its arguments and returns the standard deviation of the
numbers in the partially filled array. Since a partially filled array
requires two arguments, the function will actually have two formal
parameters: an array parameter and a formal parameter of type int
that gives the number of array positions used. The numbers in the
array will be of type double Embed your function in a suitable test
program.

stddev.cpp:

/**
 * @file stddev.cpp
 * @brief
 * @author syb0rg
 * @date 11/6/14
 */

#include 
#include 
#include 

constexpr int NUM_ELEM = 28;

double mean(double arr[])
{
    double sum = 0;
    for (int i = 0; i > input[i]) && file.good(); ++i);
    std::cout << stdDev(input) << std::endl;
    file.close();
}

As usual, a few points to complete what has already been said:

-
An std::ifstream is implicitly constructed with std::ios_base::in. No need to explicitly pass it to the constructor.

-
You don't need to explicitly call file.close(): since you are not using it again before the end of the scope where it has been declared, it will automagically be closed when destructed. That's the power of RAII! :D

-
You have two functions supposed to be reusable in other projects, mean and stdDev. The fixed size defined outside of the functions will prevent them from being reusable. You could use std::array instead and have this signature for mean:

template
double mean(const std::array& arr);

You could even templatize the type so that you can work with something else than double. Morevoer, using std::array would allow you to use a clean range-based for loop:

template
auto mean(const std::array& arr)
    -> T
{
    T sum{}; // generally performs zero-initialization
    for (auto&& val: arr)
    {
        sum += val;
    }
    return sum / N;
}

-
Alternatively, you could use the function above and compute the sum with std::accumulate:

template
auto mean(const std::array& arr)
    -> T
{
    return std::accumulate(std::begin(arr), std::end(arr), T{}) / N;
}

-
This one is more subtle and I like to bother people with it, but return is not a function, so don't put parenthesis around what you return. In your case (in most cases actually), it will work as expected, but if your return type is decltype(auto), then you may have surprises. Consider the following example:

auto foo()
    -> decltype(auto)
{
    int res = 0;
    // do stuff with res
    return (res);
}

Int this particular case, the compiler will deduce the return type int& because you returned a parenthesized identifier and so you will be returning a reference to a temporary, which isn't what you want (and which is really bad). Had you dropped the parenthesis around the return expression, then the deduced would have been the expected int. Bottom line: don't put parenthesis around the returned expression if you want to avoid surprises.

StackExchange Code Review Q#68894, answer score: 8