# A bag of numbers in C++ for constant time statistics queries - follow-up 2

(See the 2nd/previous iteration.)

I essentially removed all the type conversion trickery and have this:

numberbag.hpp

#ifndef CODERODDE_STAT_NUMBER_BAG
#define CODERODDE_STAT_NUMBER_BAG

#include <cmath>
#include <cstddef>
#include <iostream>
#include <initializer_list>

namespace coderodde {
namespace stat {

template<typename FloatingPoint = long double>
class number_bag final {

size_t        m_size = 0;
FloatingPoint m_sum{};
FloatingPoint m_square_sum{};

public:

number_bag() = default;

number_bag(const number_bag& other) :
m_size{other.m_size},
m_sum{other.m_sum},
m_square_sum{other.m_square_sum}
{}

number_bag& operator=(const number_bag& other) {
m_size = other.m_size;
m_sum = other.m_sum;
m_square_sum = other.m_square_sum;
return *this;
}

number_bag(number_bag&& other) :
m_size{other.m_size},
m_sum{std::move(other.m_sum)},
m_square_sum{std::move(other.m_square_sum)}
{}

number_bag& operator=(number_bag&& other) {
m_size = other.m_size;
m_sum = std::move(other.m_sum);
m_square_sum = std::move(other.m_square_sum);
return *this;
}

number_bag(std::initializer_list<FloatingPoint> init_lst) {
for (auto fp : init_lst) {
}
}

++m_size;
m_sum += num;
m_square_sum += num * num;
}

void remove(FloatingPoint num) noexcept {
--m_size;
m_sum -= num;
m_square_sum -= num * num;
}

void clear() noexcept {
m_size = 0;
m_sum = FloatingPoint{};
m_square_sum = FloatingPoint{};
}

size_t size() const noexcept {
return m_size;
}

FloatingPoint average() const noexcept {
return m_sum / m_size;
}

FloatingPoint variance() const noexcept {
FloatingPoint step1 = m_square_sum - (m_sum * m_sum) / m_size;
return step1 / (m_size - 1);
}

FloatingPoint standard_deviation() const noexcept {
return std::sqrt(variance());
}

number_bag operator+(FloatingPoint fp) const noexcept {
number_bag ret(*this);
return ret;
}

number_bag operator-(FloatingPoint fp) const noexcept {
number_bag ret(*this);
ret.remove(fp);
return ret;
}

number_bag& operator+=(FloatingPoint num) noexcept {
return *this;
}

number_bag& operator-=(FloatingPoint num) noexcept {
remove(num);
return *this;
}
};

template<typename FloatingPoint>
auto operator+(FloatingPoint fp, number_bag<FloatingPoint>& bag) noexcept {
number_bag<FloatingPoint> ret(bag);
return ret;
}

template<typename FloatingPoint>
auto operator-(FloatingPoint fp, number_bag<FloatingPoint>& bag) noexcept {
number_bag<FloatingPoint> ret(bag);
ret.remove(fp);
return ret;
}

template<typename Num>
std::ostream& operator<<(std::ostream& out, number_bag<Num> const &bag)
{
out << "[size=" << bag.size() << ", ave=" << bag.average() << ", var="
<< bag.variance() << ", std=" << bag.standard_deviation() << "]";
return out;
}

} // end of namespace coderodde::stat
} // end of namespace coderodde

#endif // CODERODDE_STAT_NUMBER_BAG


main.cpp

#include "numberbag.hpp"
#include <iostream>

int main() {
using coderodde::stat::number_bag;
using namespace std;

number_bag<float> bag;

cout << "bag:  " << bag << '\n';

auto bag2 = bag - 1.0l;

cout << "bag:  " << bag << '\n';
cout << "bag2: " << bag2 << '\n';

auto bag3 = 1.0f + bag2;

cout << "bag3: " << bag3 << '\n';

bag.clear();

cout << "bag:  " << bag << '\n';

bag += 2.0f;
bag += 7.0f;

cout << "bag:  " << bag << '\n';

bag2 = {1, 1, 3.0};
cout << "bag2: " << bag2 << '\n';
return 0;
}


Critique request

1. Why do you re-implement the default copy and move constructors and assignments? Just let the compiler generate them for you.

2. Mark operator<< as noexcept. main too if you wish, although not required, as if main throws an exception, the program terminates no matter what.

3. You may omit the return 0; in main, the compiler will add one for you if you omit it.

4. To avoid copies, use auto&& instead of auto:

for (auto&& fp : init_lst) {
}

5. With C++17, you would be able to check if operator+ on FloatingPoint will throw or not with the std::is_nothrow_invocable type trait. That will enable you to enable or disable noexcept on functions that call operator+.

In the mean time, you can either leave them noexcept, which means that any FloatingPoint::operator+ can't throw, or remove it, to let users add a throwing operator+. The same goes for any functions/operators that you're either not sure about, or want to require the type to not throw.

6. You could add some comparison operators to compare number_bag objects.

• Point 1. Will it be sufficient to declare all 5 as = default? Apr 27, 2017 at 16:26
• Hmm. It did actually. Apr 27, 2017 at 16:30
• @coderodde Well, yes. But you can remove them (except for the default constructors if course), as they are not implicitly deleted. Apr 27, 2017 at 16:30
• Also, default move constructor moves everything to move, whereas the default copy constructor copies every field? Apr 27, 2017 at 16:33
• @coderodde Exactly Apr 27, 2017 at 16:38

# Consider adding an insert iterator

You don't know how this class is going to be used, but there's a good chance that somebody will want to populate it using one of the standard algorithms, such as std::transform or std::copy.

You can make that easier to do by providing a suitable iterator. A simple way to do so is to write push_back() (and possibly also push_front() and insert()). With such a method, std::back_inserter() can be used; it will to return a std::back_insert_iterator<number_bag>, which will call your push_back() as needed, without requiring any further work on your part.

void push_back(const FloatingPoint& value) noexcept
{
}


Alternatively, just rename add() - you might then want to change remove() to pop_back().

While you might just want to use the default move constructor and move assignment, you should mark them noexcept Otherwise the compiler needs to wrap those with exception handling code.

• Nice one! Never thought about what difference noexcept has. Keep 'em coming! May 18, 2017 at 15:09

Your code suffers from he numerical precision problem I described in my answer to the previous iteration. To avoid this, we can store the current mean and n times the current variance, and use Welford's method to update these when numbers are added to the bag.

Here's an outline of this approach:

#include <limits>

template<typename T>
requires std::numeric_limits<T>::has_quiet_NaN
class SimpleStatsBag
{
static constexpr auto nan = std::numeric_limits<T>::quiet_NaN();

std::size_t count = 0;
value_type current_mean = 0;
variance_type current_nvar = 0;     // count times the current variance

public:

// Accessors for the statistical properties
std::size_t size() const { return count; }

T mean() const { return count ? current_mean : nan; }

T population_variance() const { return count ? current_nvar / count : nan; }
T sample_variance() const { return count > 1 ? population_variance() * count / (count - 1) : nan; }

// Mutators - add and remove values
void insert(value_type value)
{
auto const old_mean = current_mean;
current_mean += (value - current_mean) / ++count;
current_nvar += (value - current_mean) * (value - old_mean);
}

void remove(value_type value)
{
auto const old_mean = current_mean;
if (count) {
current_mean -= (value - current_mean) / --count;
current_nvar -= (value - current_mean) * (value - old_mean);
}
}
};


(I've used Concepts to constrain the template above; just remove the requires line if your compiler doesn't support that yet).