1
\$\begingroup\$

The following code was inspired from a tutorial I found online:

#include <cstdio>
#include <cstdarg>

double average_function(const int num_elements, ...)
{
    va_list elements;
    double sum = 0.0;

    va_start(elements, num_elements);
    for(int i = 0; i < num_elements; ++i) {
        sum += va_arg(elements, double);
    }
    va_end(elements);
    return sum / num_elements;
}

int main( int argc, char ** argv ) {

    printf("Average: %lf", average_function(5, 25.0, 35.7, 50.1, 127.6, 75.0));
    return 0;

}

I believe that the same can be done in many different ways.

Is there a better way to do the same without having as first arguments of the function the number of elements we want to sum? For example having instead a function to compute the average that have in its input only the set of elements we want to take into account without a their length?

\$\endgroup\$
5
\$\begingroup\$

One possibility would be to use an std::initializer_list:

#include <initializer_list>
#include <numeric>
#include <iostream>

double average(std::initializer_list<double> input) { 
    double sum = std::accumulate(input.begin(), input.end(), 0.0);
    return sum / input.size();
}

int main() { 
     std::cout << "Average: " << average({1, 2, 3, 4, 5, 6, 7, 8});
}

You could have the function take (for example) a vector instead, and it'd still work pretty much the same way (because vector already has a constructor that takes an initializer_list). Either way works out pretty much the same way though.

If you want to use just a list of numbers rather than having to add the {} around the list to force it into an initializer_list you can add var-args template function to wrap the call.

template<typename... Args>
double average(Args... args)
{
    return average({args...});
}

Looking at your code, a few things nearly jump out. First and foremost, you're using a C-style variable parameter list. This is mostly frowned upon in C++ for a number of reasons such as lack of type-safety. For example, if you'd accidentally passed your 25.0 as just 25, it'd be passed as an int instead of a double, and the whole thing would break.

The second is similar: using printf to print out the result has exactly the same problem for exactly the same reason (printf also uses a C-style variable parameter list).

Third, related to those but technically a separate issue (since it's about values, not types) is the problem you've apparently already noticed: it requires you to count the number of parameters. If you make a mistake, you get bad results (and if you tell it you're passing more parameters than you really do, undefined behavior).

Simply put, although your code won't compile as-is with a C compiler, it's still basically C-style code. It does nothing to fit with (or take advantage of) anything in C++ that wasn't a part of C 20+ years ago. If used correctly, it works, but it's all too easy to use it incorrectly.

\$\endgroup\$
  • \$\begingroup\$ Damm you beat me to it. \$\endgroup\$ – Martin York Feb 6 '16 at 18:07
  • \$\begingroup\$ Added a wrapper to make it more intuitive to use. \$\endgroup\$ – Martin York Feb 6 '16 at 18:12
  • \$\begingroup\$ PS. Initializer_list has a size method so you don't need to use std::distance() \$\endgroup\$ – Martin York Feb 6 '16 at 18:12
  • \$\begingroup\$ @LokiAstari: Yeah, I see that--thank you. I thought about that too, but hadn't gotten a chance to test the code to make sure I got it right. \$\endgroup\$ – Jerry Coffin Feb 6 '16 at 18:12
  • \$\begingroup\$ @LokiAstari: Oh, of course. I'd better get offline and get some exercise or something--I'm clearly not entirely awake yet. \$\endgroup\$ – Jerry Coffin Feb 6 '16 at 18:14
2
\$\begingroup\$

I think this question is more appropriate for StackOverflow, but it can also be answered here:

1. Number of parameters

  • one way it to provide the number of elements and have them all in a int* array

  • (your solution) use variable number of parameters is not recommended as explained here)

  • use standard array which encapsulate your values in a more decent container that also provides the length

2. Performance consideration

This applies only for very large arrays for which normal summing might not be fast enough. Your algorithm executes in O(n), but the effort can be split on multiple threads by using a divide and impera approach.

For your simple program, I recommend to get started with std:array.

\$\endgroup\$
  • \$\begingroup\$ Passing an int* is a terrible idea many better techniques. \$\endgroup\$ – Martin York Feb 6 '16 at 18:16
  • \$\begingroup\$ To use the divide method is only useful for an absolutely huge number of values. The cost of spinning up separate threads is not trivial. Since the question is about passing arguments as parameters to a function this is not going to happen. \$\endgroup\$ – Martin York Feb 6 '16 at 18:18
  • \$\begingroup\$ @Loki Astari - you are right. It was mentioned only for completeness. Probably, it would make sense in a SIMD or similar architecture. \$\endgroup\$ – Alexei Feb 6 '16 at 19:10

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.