5
\$\begingroup\$

I want this to be similar to the STL algorithms but I don't find it elegant nor concise at all:

#include <iostream>
#include <iterator>
#include <vector>
#include <algorithm>

using E = double;
template <typename IT>
E std_dev(IT begin, IT end){
    auto N = std::distance(begin, end);
    E average = std::accumulate(begin, end, E()) / N;
    auto sum_term = [average](E init, E value)-> E{ 
        return init + (value - average)*(value - average);
    };
    E variance = std::accumulate(begin,  end, E(), sum_term);
    return std::sqrt(variance * 1.0 / (N - 1));
}


int main(){
    std::vector<double> stuff {3.5, 3.4, 3.6, 3.9, 3.5, 3.5, 3.5, 3.5, 3.5};
    std::cout << std_dev(stuff.begin(), stuff.end()) << "\n";
}
\$\endgroup\$
3
\$\begingroup\$

Firstly, make it correct.

N is integral, you could make it E so you don't accidentally do integer arithmetic.

N-1 is wrong.

Rename average to mean.

Don't hardcode E.

You get:

template <typename It, typename E = typename std::iterator_traits<It>::value_type>
E std_dev(It begin, It end){
    E N = std::distance(begin, end);
    E const mean = std::accumulate(begin, end, E()) / N;
    auto sum_term = [mean](E init, E value)-> E { return init + (value - mean)*(value - mean); };
    E variance = std::accumulate(begin, end, E(), sum_term);
    return std::sqrt(variance / N);
}

Slightly stylized, with comparison to Boost Accumulator:

Live On Coliru

#include <iostream>
#include <iterator>
#include <vector>
#include <algorithm>

#include <boost/accumulators/accumulators.hpp>
#include <boost/accumulators/statistics.hpp>

template <typename It, typename E = typename std::iterator_traits<It>::value_type, typename R = typename std::common_type<double, E>::type>
R std_dev_boost(It begin, It end){
    namespace ba = boost::accumulators;

    ba::accumulator_set<R, ba::stats<ba::tag::variance> > accu;
    std::for_each(begin, end, std::ref(accu));
    return std::sqrt(ba::variance(accu));
}

template <typename It, 
    typename E = typename std::iterator_traits<It>::value_type, 
    typename R = typename std::common_type<double, E>::type>
R std_dev(It b, It e)
{
    R N          = std::distance(b, e);
    R const mean = std::accumulate(b, e, R{}) / N;
    R variance   = std::accumulate(b, e, R{}, [mean](R a, E v)-> R { return a + (v-mean)*(v-mean); });
    return std::sqrt(variance / N);
}

int main(){
    std::vector<int> stuff {35, 34, 36, 39, 35, 35, 35, 35, 35};
    std::cout << std_dev_boost(stuff.begin(), stuff.end()) << "\n";
    std::cout << std_dev      (stuff.begin(), stuff.end()) << "\n";
}

Prints

1.34256
1.34256
\$\endgroup\$
  • \$\begingroup\$ @CaptainGiraffe Mmm. I was mistaken there. The N-1 seems to be the only mistake that made the outcome wrong :) \$\endgroup\$ – sehe Mar 19 '16 at 0:42
  • \$\begingroup\$ Ah. Well, see my update for how to derive a proper result type that works even if the input is an integer container. \$\endgroup\$ – sehe Mar 19 '16 at 0:49
  • 1
    \$\begingroup\$ The N-1 isn't wrong. It's just computing a different standard deviation. To be specific, if you're computing the standard deviation of "the universe", you use N. If you're computing the standard deviation of a sample, you use N-1. \$\endgroup\$ – Jerry Coffin Mar 19 '16 at 1:51
  • \$\begingroup\$ @JerryCoffin Yeah, that was clear to me since Giraffe told me :) I was content contributing the style feedback \$\endgroup\$ – sehe Mar 19 '16 at 2:21
-2
\$\begingroup\$

A simple conciseness improvement would be to inline non-informative variables:

template <typename IT>
E std_dev(IT begin, IT end) {
    auto N = std::distance(begin, end);
    E variance = std::accumulate(begin,  end, E{},
        [average=std::accumulate(begin, end, E{}) / N](E init, E value) -> E
    {
        return init + (value - average)*(value - average);
    });
    return std::sqrt(variance * 1.0 / (N - 1));
}

average was moved into the lambda capture (C++14(?)) and the lambda itself was passed directly to std::accumulate.

\$\endgroup\$
  • \$\begingroup\$ You left in all the bugs. \$\endgroup\$ – sehe Mar 19 '16 at 0:36
  • \$\begingroup\$ @sehe No, I didn't remove them. I'm here for conciseness (as stated in the answer), not for correctness. \$\endgroup\$ – набиячлэвэлиь Mar 19 '16 at 0:39
  • 3
    \$\begingroup\$ The main point of programming is to make it easier to read not more concise. \$\endgroup\$ – Martin York Mar 19 '16 at 0:51
  • 2
    \$\begingroup\$ If you want it concise, use valarray. stackoverflow.com/a/1723071/179910 \$\endgroup\$ – Jerry Coffin Mar 19 '16 at 1:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.