patterncppMinor
Generic sliding average in C++
Viewed 0 times
genericslidingaverage
Problem
I tried hard to come up with a function template computing a sliding average. A sliding average over data \$x_1, x_2, \dots, x_n\$ with window length \$k \leq n\$ is a sequence \$y_1, y_2, \dots, y_{n - k + 1}\$, where
$$y_i = \frac{1}{k}\sum_{j = 0}^{k - 1} x_{i + j}.$$
Here is my attempt:
mymath.h
main.cpp
Critique request
I would like to receive comments regarding how to make my implementation more generic, and how to make it more idiomatic. Other comments are welcome as well.
$$y_i = \frac{1}{k}\sum_{j = 0}^{k - 1} x_{i + j}.$$
Here is my attempt:
mymath.h
#ifndef MYMATH_H
#define MYMATH_H
#include
#include
#include
template
void sliding_average(ForwardIterator begin,
ForwardIterator end,
OutputIterator output,
size_t window_length)
{
if (window_length == 0)
{
std::stringstream ss;
ss ::value_type;
ForwardIterator finger = begin;
T sum {};
size_t count = 0;
while (finger != end and count < window_length)
{
sum += *finger++;
count++;
}
if (count < window_length)
{
std::stringstream ss;
ss << "The length of the range (";
ss << count;
ss << ") is too short. Must be at least ";
ss << window_length;
throw std::runtime_error{ss.str()};
}
*output++ = sum / window_length;
ForwardIterator window_tail = begin;
while (finger != end)
{
sum -= *window_tail++;
sum += *finger++;
*output++ = sum / window_length;
}
}
#endif // MYMATH_Hmain.cpp
#include "mymath.h"
#include
#include
using std::cout;
using std::endl;
using std::begin;
using std::end;
int main(int argc, const char * argv[]) {
float input[15];
for (size_t i = 0; i < 15; ++i)
{
input[i] = i + 1;
}
float output[11];
sliding_average(begin(input), end(input), begin(output), 5);
for (auto& a : output)
{
cout << a << " ";
}
cout << endl;
return 0;
}Critique request
I would like to receive comments regarding how to make my implementation more generic, and how to make it more idiomatic. Other comments are welcome as well.
Solution
Very good. There are small things that could be improved.
It is possible to give
Usually those are called
Well, the result of
I think that using
Some caveats:
Currently if
It is possible to give
window_length type std::iterator_traits::difference_type. I think std::size_t is fine for most cases.ForwardIterator begin,
ForwardIterator endUsually those are called
first and last.if (window_length == 0)
{
std::stringstream ss;
ss << "Bad window_length: ";
ss << window_length;
throw std::runtime_error{ss.str()};
}Well, the result of
ss.str() is obvious :) It is possible to write constexpr there, or throw the string right into the constructor. Also, runtime_error is a good fit, but it has child called invalid_argument, which perfectly matches the case.if (count < window_length)
{
std::stringstream ss;
ss << "The length of the range (";
ss << count;
ss << ") is too short. Must be at least ";
ss << window_length;
throw std::runtime_error{ss.str()};
}I think that using
std::stringstream is an overkill here. Throwing just "The length of the range is too short. It must be at least of length window_length" is pretty good by itself, since most IDEs will probably stop execution, so that programmers could have a look. Even if they had a catch for this, they would need to parse a string to be actually able to do something. I don't think it worth the troubles it brings.Some caveats:
Currently if
T = int the algorithm is going to produce somewhat incorrect results. May be you could write something like warning mechanism that will warn when integer type is used. I would consider #pragma message("your warning message here"). It might get portability problems but the code will still compile since unrecognized #pragmas are ignored.Code Snippets
ForwardIterator begin,
ForwardIterator endif (window_length == 0)
{
std::stringstream ss;
ss << "Bad window_length: ";
ss << window_length;
throw std::runtime_error{ss.str()};
}if (count < window_length)
{
std::stringstream ss;
ss << "The length of the range (";
ss << count;
ss << ") is too short. Must be at least ";
ss << window_length;
throw std::runtime_error{ss.str()};
}Context
StackExchange Code Review Q#149787, answer score: 3
Revisions (0)
No revisions yet.