Skip to main content
Return a NaN for empty bags; and fix a mis-copy
Source Link
Toby Speight
  • 88.4k
  • 14
  • 104
  • 327

#Headers and namespaces We don't use <sstring>; thatany string-stream, so #include <sstream> line can be removed.

We should pass the vector by reference to a const object, as we don't want to make a copy or to modify the value.

Instead of returning zero when there are no members, perhaps we should return a NaN value (which is more consistent with arithmetic 0.0 / 0):

if (numbers.empty())
    return std::numeric_limits<double>::quiet_NaN();
double computeSampleVariance(const double mean, const std::vector<int>& numbers)
{
    if (numbers.size() <= 1u)
        return 0;std::numeric_limits<double>::quiet_NaN();

    auto add_square = [mean](double sum, int i)
        {
            auto d = i - mean;
            return sum + d*d;
        };
    double total = std::accumulate(numbers.begin(), numbers.end(), 0.0, add_square);
    return total / (numbers.size() - 1);
}
#include <algorithm>
#include <cmath>
#include <fstream>
#include <iostream>
#include <iterator>
#include <limits>
#include <numeric>
#include <vector>

std::vector<int> readFile(const std::string& filePath)
{
    std::ifstream in_file(filePath);
    std::istream_iterator<int> start{in_file}, end;
    std::vector<int> numbers;
    std::copy(start, end, std::back_inserter(numbers));
    return numbers;
}

double computeMean(const std::vector<int>& numbers)
{
    if (numbers.empty())
        return 0;std::numeric_limits<double>::quiet_NaN();

    return std::accumulate(numbers.begin(), numbers.end(), 0.0) / numbers.size();
}

double computeSampleVariance(const double mean, const std::vector<int>& numbers)
{
    if (numbers.size() <= 1u)
        return 0;std::numeric_limits<double>::quiet_NaN();

    auto const add_square = [mean](double sum, int i) {
        auto d = i - mean;
        return sum + d*d;
    };
    double total = std::accumulate(numbers.begin(), numbers.end(), 0.0, add_square);
    return total / (numbers.size() - 1);
}

int main()
{
#ifdef TEST
    const std::vector<int> numbers = { -2, -1, 1, 2, 100000-2, 100000-1, 100000+1, 100000+2};
#else
    std::cout << "Please enter the file path :" << std::endl;
    std::string filePath;
    std::cin >> filePath;
    const std::vector<int> numbers = readFile(filePath);
#endif

    double mean = computeMean(numbers);
    double variance = computeSampleVariance(mean, numbers);
    double standardDeviation = std::sqrt(variance);

    std::cout << numbers.size() << " numbers : ";
    auto separator = "";
    for (int number: numbers) {
        std::cout << separator << number;
        separator = ", ";
    }
    std::cout << std::endl;

    std::cout << "Mean : " << std::to_string(mean)
              << "Variance : " << std::to_string(variance)
              << "Standard Deviation : " << std::to_string(standardDeviation) << std::endl;
    return 0;
}

#Headers and namespaces We don't use <sstring>; that #include line can be removed.

We should pass the vector by reference to a const object, as we don't want to make a copy or to modify the value.

double computeSampleVariance(const double mean, const std::vector<int>& numbers)
{
    if (numbers.size() <= 1u)
        return 0;

    auto add_square = [mean](double sum, int i)
        {
            auto d = i - mean;
            return sum + d*d;
        };
    double total = std::accumulate(numbers.begin(), numbers.end(), 0.0, add_square);
    return total / (numbers.size() - 1);
}
#include <algorithm>
#include <cmath>
#include <fstream>
#include <iostream>
#include <iterator>
#include <numeric>
#include <vector>

std::vector<int> readFile(const std::string& filePath)
{
    std::ifstream in_file(filePath);
    std::istream_iterator<int> start{in_file}, end;
    std::vector<int> numbers;
    std::copy(start, end, std::back_inserter(numbers));
    return numbers;
}

double computeMean(const std::vector<int>& numbers)
{
    if (numbers.empty())
        return 0;

    return std::accumulate(numbers.begin(), numbers.end(), 0.0) / numbers.size();
}

double computeSampleVariance(const double mean, const std::vector<int>& numbers)
{
    if (numbers.size() <= 1u)
        return 0;

    auto const add_square = [mean](double sum, int i) {
        auto d = i - mean;
        return sum + d*d;
    };
    double total = std::accumulate(numbers.begin(), numbers.end(), 0.0, add_square);
    return total / (numbers.size() - 1);
}

int main()
{
#ifdef TEST
    const std::vector<int> numbers = { -2, -1, 1, 2, 100000-2, 100000-1, 100000+1, 100000+2};
#else
    std::cout << "Please enter the file path :" << std::endl;
    std::string filePath;
    std::cin >> filePath;
    const std::vector<int> numbers = readFile(filePath);
#endif

    double mean = computeMean(numbers);
    double variance = computeSampleVariance(mean, numbers);
    double standardDeviation = std::sqrt(variance);

    std::cout << numbers.size() << " numbers : ";
    auto separator = "";
    for (int number: numbers) {
        std::cout << separator << number;
        separator = ", ";
    }
    std::cout << std::endl;

    std::cout << "Mean : " << std::to_string(mean)
              << "Variance : " << std::to_string(variance)
              << "Standard Deviation : " << std::to_string(standardDeviation) << std::endl;
    return 0;
}

#Headers and namespaces We don't use any string-stream, so #include <sstream> can be removed.

We should pass the vector by reference to a const object, as we don't want to make a copy or to modify the value.

Instead of returning zero when there are no members, perhaps we should return a NaN value (which is more consistent with arithmetic 0.0 / 0):

if (numbers.empty())
    return std::numeric_limits<double>::quiet_NaN();
double computeSampleVariance(const double mean, const std::vector<int>& numbers)
{
    if (numbers.size() <= 1u)
        return std::numeric_limits<double>::quiet_NaN();

    auto add_square = [mean](double sum, int i)
        {
            auto d = i - mean;
            return sum + d*d;
        };
    double total = std::accumulate(numbers.begin(), numbers.end(), 0.0, add_square);
    return total / (numbers.size() - 1);
}
#include <algorithm>
#include <cmath>
#include <fstream>
#include <iostream>
#include <iterator>
#include <limits>
#include <numeric>
#include <vector>

std::vector<int> readFile(const std::string& filePath)
{
    std::ifstream in_file(filePath);
    std::istream_iterator<int> start{in_file}, end;
    std::vector<int> numbers;
    std::copy(start, end, std::back_inserter(numbers));
    return numbers;
}

double computeMean(const std::vector<int>& numbers)
{
    if (numbers.empty())
        return std::numeric_limits<double>::quiet_NaN();

    return std::accumulate(numbers.begin(), numbers.end(), 0.0) / numbers.size();
}

double computeSampleVariance(const double mean, const std::vector<int>& numbers)
{
    if (numbers.size() <= 1u)
        return std::numeric_limits<double>::quiet_NaN();

    auto const add_square = [mean](double sum, int i) {
        auto d = i - mean;
        return sum + d*d;
    };
    double total = std::accumulate(numbers.begin(), numbers.end(), 0.0, add_square);
    return total / (numbers.size() - 1);
}

int main()
{
#ifdef TEST
    const std::vector<int> numbers = { -2, -1, 1, 2, 100000-2, 100000-1, 100000+1, 100000+2};
#else
    std::cout << "Please enter the file path :" << std::endl;
    std::string filePath;
    std::cin >> filePath;
    const std::vector<int> numbers = readFile(filePath);
#endif

    double mean = computeMean(numbers);
    double variance = computeSampleVariance(mean, numbers);
    double standardDeviation = std::sqrt(variance);

    std::cout << numbers.size() << " numbers : ";
    auto separator = "";
    for (int number: numbers) {
        std::cout << separator << number;
        separator = ", ";
    }
    std::cout << std::endl;

    std::cout << "Mean : " << std::to_string(mean)
              << "Variance : " << std::to_string(variance)
              << "Standard Deviation : " << std::to_string(standardDeviation) << std::endl;
    return 0;
}
Bounty Awarded with 50 reputation awarded by IEatBagels
Add reworked code
Source Link
Toby Speight
  • 88.4k
  • 14
  • 104
  • 327

#My version

#include <algorithm>
#include <cmath>
#include <fstream>
#include <iostream>
#include <iterator>
#include <numeric>
#include <vector>

std::vector<int> readFile(const std::string& filePath)
{
    std::ifstream in_file(filePath);
    std::istream_iterator<int> start{in_file}, end;
    std::vector<int> numbers;
    std::copy(start, end, std::back_inserter(numbers));
    return numbers;
}

double computeMean(const std::vector<int>& numbers)
{
    if (numbers.empty())
        return 0;

    return std::accumulate(numbers.begin(), numbers.end(), 0.0) / numbers.size();
}

double computeSampleVariance(const double mean, const std::vector<int>& numbers)
{
    if (numbers.size() <= 1u)
        return 0;

    auto const add_square = [mean](double sum, int i) {
        auto d = i - mean;
        return sum + d*d;
    };
    double total = std::accumulate(numbers.begin(), numbers.end(), 0.0, add_square);
    return total / (numbers.size() - 1);
}

int main()
{
#ifdef TEST
    const std::vector<int> numbers = { -2, -1, 1, 2, 100000-2, 100000-1, 100000+1, 100000+2};
#else
    std::cout << "Please enter the file path :" << std::endl;
    std::string filePath;
    std::cin >> filePath;
    const std::vector<int> numbers = readFile(filePath);
#endif

    double mean = computeMean(numbers);
    double variance = computeSampleVariance(mean, numbers);
    double standardDeviation = std::sqrt(variance);

    std::cout << numbers.size() << " numbers : ";
    auto separator = "";
    for (int number: numbers) {
        std::cout << separator << number;
        separator = ", ";
    }
    std::cout << std::endl;

    std::cout << "Mean : " << std::to_string(mean)
              << "Variance : " << std::to_string(variance)
              << "Standard Deviation : " << std::to_string(standardDeviation) << std::endl;
    return 0;
}

#My version

#include <algorithm>
#include <cmath>
#include <fstream>
#include <iostream>
#include <iterator>
#include <numeric>
#include <vector>

std::vector<int> readFile(const std::string& filePath)
{
    std::ifstream in_file(filePath);
    std::istream_iterator<int> start{in_file}, end;
    std::vector<int> numbers;
    std::copy(start, end, std::back_inserter(numbers));
    return numbers;
}

double computeMean(const std::vector<int>& numbers)
{
    if (numbers.empty())
        return 0;

    return std::accumulate(numbers.begin(), numbers.end(), 0.0) / numbers.size();
}

double computeSampleVariance(const double mean, const std::vector<int>& numbers)
{
    if (numbers.size() <= 1u)
        return 0;

    auto const add_square = [mean](double sum, int i) {
        auto d = i - mean;
        return sum + d*d;
    };
    double total = std::accumulate(numbers.begin(), numbers.end(), 0.0, add_square);
    return total / (numbers.size() - 1);
}

int main()
{
#ifdef TEST
    const std::vector<int> numbers = { -2, -1, 1, 2, 100000-2, 100000-1, 100000+1, 100000+2};
#else
    std::cout << "Please enter the file path :" << std::endl;
    std::string filePath;
    std::cin >> filePath;
    const std::vector<int> numbers = readFile(filePath);
#endif

    double mean = computeMean(numbers);
    double variance = computeSampleVariance(mean, numbers);
    double standardDeviation = std::sqrt(variance);

    std::cout << numbers.size() << " numbers : ";
    auto separator = "";
    for (int number: numbers) {
        std::cout << separator << number;
        separator = ", ";
    }
    std::cout << std::endl;

    std::cout << "Mean : " << std::to_string(mean)
              << "Variance : " << std::to_string(variance)
              << "Standard Deviation : " << std::to_string(standardDeviation) << std::endl;
    return 0;
}
Mention the single-pass algorithm
Source Link
Toby Speight
  • 88.4k
  • 14
  • 104
  • 327

#Single-pass algorithm It is possible to compute the mean and variances in a single pass - but not using the (exact-arithmetic) methods you likely learnt in high school, which suffer from lack of precision with the inexact floating-point types we can use. The topic is too deep for this review, but if you research Welford's Algorithm, you will find reference implementations to guide you.

That said, for your purposes, the straightforward two-pass algorithm is probably appropriate, and it's easy to read and understand, so I wouldn't recommend changing it unless you reach a point where your input set becomes too large to hold in a vector (and even then, only if you can't read the file multiple times).

#Single-pass algorithm It is possible to compute the mean and variances in a single pass - but not using the (exact-arithmetic) methods you likely learnt in high school, which suffer from lack of precision with the inexact floating-point types we can use. The topic is too deep for this review, but if you research Welford's Algorithm, you will find reference implementations to guide you.

That said, for your purposes, the straightforward two-pass algorithm is probably appropriate, and it's easy to read and understand, so I wouldn't recommend changing it unless you reach a point where your input set becomes too large to hold in a vector (and even then, only if you can't read the file multiple times).

Source Link
Toby Speight
  • 88.4k
  • 14
  • 104
  • 327
Loading