0

I have a function:

vector<int> prime(int num, ...) {

vector<int> mas;
va_list args;
va_start(args, num);

for (int i = 0; i < num; i++) {
    int v = va_arg(args,int);
    if (isPrime(v)){
        mas.push_back(v);
    }
}
cout << endl;
va_end(args);
return mas;}

It should detected prime numbers. But when i call it, part of my numbers, don`t get over.

It looks something like this

Input: 5, 7, 10, 15, 20,12, 13,16,19

Numbers what cout returns in the loop: 7,7

Help pls!

pmg
  • 106,608
  • 13
  • 126
  • 198
nyar
  • 37
  • 5
  • You call `va_arg(args, int)` 3*n times. How is it supposed to know that the first three calls should return the first argument, the second three should return the second argument, and so on? It burns through all arguments in n/3 iterations, and then UB happens. You need to call it one per iteration, and save the result to a variable. – HolyBlackCat Dec 26 '21 at 17:05
  • 4
    If possible, stay away from C-style variadic arguments. Pass `const std::vector &` as a parameter (or, in C++20, `std::span`), or, if you absolutely want the same call syntax you have now, use variadic templates. – HolyBlackCat Dec 26 '21 at 17:07
  • i remake my code and now it call va_arg one per iteration, but my problem haven`t fixed – nyar Dec 26 '21 at 17:15
  • 1
    Works on my end, after I change the code as I suggested. Please add a [mcve] for the new code, and explain how exactly it doesn't work. – HolyBlackCat Dec 26 '21 at 17:18

2 Answers2

2

First of all Variadic arguments from C are considered a bad practice in C++ and should be avoided.

There are plenty of better C++ solutions which are able to replace this C feature.

Old fashioned std::vector:

std::vector<int> filterPrimes(std::vector<int> a) {
    auto end = std::remove_if(a.begin(), a.end(), [](auto x) {
       return !isPrime(x)
    };
    a.remove(end, a.end());
    return a;
}

Or std::initializer_list

std::vector<int> filterPrimes(std::initializer_list<int> l) {
    std::vector<int> r;
    std::copy_if(l.begin(), l.end(), std::back_inserter(r), isPrime);
    return r;
}

Or Variadic templates, or template with iterator ranges, or ... .

Marek R
  • 32,568
  • 6
  • 55
  • 140
1

If the arguments are all of the same type, then just pass them in as, say, a vector.

#include <vector>
#include <iostream>
using namespace std;


void func(const vector<int>& args)
{
    for (size_t i = 0; i < args.size(); i++)
        cout << args[i] << endl;
}


int main(void)
{
    vector<int> v = { 5, 7, 10, 15, 20, 12, 13, 16, 19 };

    func(v);

    return 0;
}
shawn_halayka
  • 96
  • 1
  • 1
  • 9
  • 1
    In task said that i must use a function with a variable number of parameters. – nyar Dec 26 '21 at 17:08
  • No, it's not @shawn. See https://stackoverflow.com/a/1056442 for example. –  Dec 26 '21 at 17:12
  • 1
    There is no reason to use a C feature when C++ is the target. Same goes for PRNG and timing features -- the C++ versions are so much better. – shawn_halayka Dec 26 '21 at 17:13
  • 1
    In C++, this can be accomplished with a parameter pack. On the clunkier side (for a free function call, anyway), `std::initializer_list`. I also believe that `va_arg` is the only reasonable way to do this at runtime if the types differ. – sweenish Dec 26 '21 at 17:18