how should I remove variadic functions?

Question

I read that variable argument functions are no good coding.

I have a very old framework with some variable argument functions I want to remove the variable arguments keeping the debugging function.

DEBUG(wchar_t* text, ...)

This debug function calls a printf-like function using the same string sintaxis %d,%f... etc.

What should be the right approach?

Different question with [same answer](https://stackoverflow.com/a/29326784/1362568) — Mike Kinghan, Nov 09 '18 at 11:21
I would not change that. Functions with variadic parameter lists are fine for printing, logging etc. Unless you want to change it to C++ and a stream-like interface. Which, of course, would be a major code change. — Rene, Nov 09 '18 at 11:21
You *could* do a whole bunch of re-writing such that all your format strings are changed to be `debug_stream << "Whatever" << args << "here" << etc;`, but it doesn't seem worthwhile. "are no good coding" isn't terribly persuasive against "currently works fine" — Caleth, Nov 09 '18 at 11:24
@caleth the main problem here is that a bug was found in the inner printf-like function when parsing the arguments, so, as is planned to refactor it, I was willing to "C++"-cify it. — diego.martinez, Nov 09 '18 at 11:35
Note that you cannot convert this directly to a `std::cout`-based solution. All the existing formats will be using `%s`, as the question explicitly states. `std::cout` cannot do anything with that. +1 for Matthieu Brucher; Boost.Format **does** understand `%s`. — MSalters, Nov 09 '18 at 12:19

score 2 · Answer 1 · answered Nov 09 '18 at 11:30

Since you marked the question c++, you could make a new class/function based on c++ streams for instance - perhaps even changing the 'old' system to use your new one. Then over time migrate towards that system and perhaps at some point you could get a rid of the old (the 'DEBUG' in your case).

bolov · Accepted Answer · 2018-11-09T12:31:26.347

Surprisingly I would recommend to leave the debugging function as it is, unless you are willing to change the entire interface to the C++ way of IO, aka streams. If you are going to use printf and printf syntax then let it as it is. Otherwise modernize it all the way.

For instance let's stay your implementation is somewhere along the lines:

void dbg(char* txt, ...)
{
    va_list args;
    va_start(args, txt);
    vprintf(txt, args);
    va_end(arts);
}

Yes, there are options to get rid of the variadic, but there is 0 gain in doing so if you are going to keep the printf family syntax:

template <class... Args>
auto dbg(const char* fmt, const Args&... args)
{
    printf(fmt, args...);
}

Then you realize that char* is more C than C++ and you change to this:

template <class... Args>
auto dbg(const std::string& fmt, const Args&... args)
{
    printf(fmt.c_str(), args...);
}

Then you realize that printf is more C than C++ and the option now is to get rid of printf and to scrap this altogether.

This question How to make a variadic macro for std::cout? shows you a way you could do that if you are still set on a function:

template<typename ...Args>
void log(Args && ...args)
{
    (std::cout << ... << args);
}

Another option is to make something like this:

log << "this is the " << i << " log";

But it's not trivial to make it add a newline at the end.

In the end, the best solution I think is to use a logging library.

question, is the va_end unnecesary in the first code? – diego.martinez Nov 09 '18 at 11:55 — diego.martinez, Nov 09 '18 at 11:55
Error on my part. Corrected and thank you. – bolov Nov 09 '18 at 12:31 — bolov, Nov 09 '18 at 12:31

score 1 · Answer 3 · answered Nov 09 '18 at 13:43

I agree @bolov recommendation to leave the debugging function as it is. However you may play with std::initializer_list and std::variant (since C++17) types. Below is a small example that doesn't yet process the format specifiers but can give some ideas to evolve the approach.

#include <iostream>
#include <cstdlib>
#include <variant>

typedef std::variant<std::string, int, float, bool> DebugOutParam;
std::ostream& operator << (std::ostream& os, const DebugOutParam& v)  
{  
    if (std::holds_alternative<std::string>(v))
        os << std::get<std::string>(v);
    else if (std::holds_alternative<int>(v))
        os << std::get<int>(v);
    else if (std::holds_alternative<float>(v))
        os << std::get<float>(v);
    else if (std::holds_alternative<bool>(v))
        os << (std::get<bool>(v) ? "true" : "false");
    else
        os << "?Unsupported?";
    return os;  
}
typedef std::initializer_list<DebugOutParam> DebugOutParams;

void dbg(std::string fmt, DebugOutParams l)
{
    std::cout << fmt << ": ";
    for (DebugOutParams::const_iterator it = l.begin(); it != l.end(); it++)
    {
        DebugOutParam v = *it;
        std::cout << (it == l.begin() ? "" : ", ") << v;
    }
    std::cout << std::endl;
}

int main()
{
   dbg("Test", {123, std::string("456"), true, static_cast<float>(456.789)});
}

Output

Test: 123, 456, true, 456.789

how should I remove variadic functions?

3 Answers3