Use const-references when you only read an argument
std::vector <long double> SMA(std::vector <long double> &ARR, int lag) {
A signature like this has the same effect as a coffee brewed by using energy drinks instead of water: it immediately gets flagged and asks for special care and awareness.
Why? Because we have a non-const reference here, ARR. A non-const reference indicates that we will use that argument as output; the function is able to communicate the changes within this functions back to the outside not only via its return value, but also via this argument.
This unfortunately also means that we cannot use temporary values, for example, which is a pitty. But if we take a look at the inner function, we see that nowhere ARR is modified. This means that your real function signature should be
// vvvvv
std::vector <long double> SMA(const std::vector <long double> &ARR, int lag) {
This will also ensure that you don't change ARR in this function by accident later on, as the compiler will (rightfully) yell at you.
Use a single scalar value for accumulation
std::vector<long double> temp_vector;
for (size_t n = 0; n < lag; ++n) {
temp_vector.push_back(ARR[i - n]);
}
long double sum = accumulate(temp_vector.begin(), temp_vector.end(), 0.0, std::plus<long double>());
long double average = sum / lag;
Let's have a look at the accumulate line and std::accumulate's documentation: the effects of accumulate are just like
sum = (... ((temp_vector[0] + temp_vector[1]) + temp_vector[2]) + ... ) + temp_vector[lag]
But since the temp_vector elements just stem from the original ARR, we can use that one instead:
sum = (... ((ARR[i] + ARR[i - 1]) + ARR[i - 2]) + ... ) + ARR[i - lag]
At that point, we can just get rid of temp_vector and immediately sum, as we keep the associativity of std::accumulate:
long double sum = 0.0;
for (size_t n = 0; n < lag; ++n) {
sum += ARR[i - n];
}
const long double average = sum / lag;
But you can still use std::accumulate if you want to:
const long double sum = std::accumulate(ARR.begin() + (i - n)
,ARR.begin() + i
,0.0
,std::plus<long double>());
however, that can yield other results, since we're now summing in reverse order. Which brings us to the next topic.
Prefer easier ranges
Let's get back to the previous version:
for (size_t n = 0; n < lag; ++n) {
temp_vector.push_back(ARR[i - n]);
}
First of all: if you don't need to traverse ARR backwards, then don't. Handling indices can be quite a pain. It's a lot easier to have ARR[i], ARR[i+1] in mind than ARR[i], ´ARR[i-1]`, not only for ourself, but also for the machine, as going forward through memory can be a performance improvement. Mind you, it can be an improvement; it depends on the actual hardware and compiler.
But we can also make it faster for humans to understand:
for (size_t n = i + 1 - lag; n <= i; ++n) {
temp_vector.push_back(ARR[n]);
}
Similarly, you might want to move the initial lag values out of your inner loop, e.g.
for (size_t i = 0; i < lag; ++i) {
sma_values.push_back(std::numeric_limits<double>::quiet_NaN());
}
for (size_t i = lag; i < ARR.size(); ++i) {
...
}
Overall
I'd also try to improve the naming, if possible. SMA might make sense to you now, but (simple_)moving_average will make sense to you even in three months. Similar, values instead of ARR might be a better name, since it's not actually an array. Remember: the compiler doesn't care about the length of your variable's names, but you might.
With this in mind, we end up with:
std::vector<long double> moving_average(const std::vector<long double> &values, size_t lag) {
std::vector<long double> averages;
for (size_t i = 0; i < lag; ++i) {
averages.push_back(std::numeric_limits<double>::quiet_NaN());
}
for (size_t i = lag; i < values.size(); ++i) {
long double sum = 0.0;
// Still traversing backwards to preserve your original
// accumulation, but you might want to introduce the other
// variants
for (size_t n = 0; n < lag; ++n) {
sum += values[i - n];
}
const long double average = sum / lag;
averages.push_back(average);
}
return averages;
}
