13

So I had this function...

virtual void CallRemoteFunction( const char* pServerGameObjectId, const char* pFunctionName, OVariant arg1 = OVariant(), OVariant arg2 = OVariant(), OVariant arg3 = OVariant(), OVariant arg4 = OVariant(), OVariant arg5 = OVariant(), OVariant arg6 = OVariant(), OVariant arg7 = OVariant(), OVariant arg8 = OVariant(), OVariant arg9 = OVariant() );

I decided to rewrite the function because, frankly, I was embarrassed by it. The function is straightforward... take a variable number of arguments of unknown type and do stuff.

I am fairly new to modern C++, so I did some searching and assumed I would find some simple/elegant new way of doing this. I imagined something like...

//hypothetical code
virtual void CallRemoteFunction( const char* pServerGameObjectId, const char* pFunctionName, ... args )
{
    std::vector<OVariant> argsArray;
    for (auto& arg : args )
    {
        argsArray.push_back(arg)
    }

    //do other stuff
 }
 //end hypothetical code

But in my searches, I could not find anything close. So could anybody kindly give me some ideas on how to refactor my original function into something cleaner or simpler? Note: the solution must be C++ 11 or older.

-Update- The function does not have to be virtual.
I want to be able to call the function like so...

CallRemoteFunction("serverID","someFunc",1,2,3);

Here is the implementation for reference

//calls function on client if this peer is a linked server or vice versa
void Peer::CallRemoteFunction( const char* pServerGameObjectId,  const char* 
pFunctionName, OVariant arg1, OVariant arg2, OVariant arg3, OVariant arg4, 
OVariant arg5, OVariant arg6, OVariant arg7, OVariant arg8, OVariant arg9 )
{
    Command command;
    command.SetObjectName( pServerGameObjectId );
    command.SetFunctionName( pFunctionName );
    command.ResetArgs();
    if ( arg1.IsValid() ){ command.PushArg( arg1 ); }
    if ( arg2.IsValid() ){ command.PushArg( arg2 ); }
    if ( arg3.IsValid() ){ command.PushArg( arg3 ); }
    if ( arg4.IsValid() ){ command.PushArg( arg4 ); }
    if ( arg5.IsValid() ){ command.PushArg( arg5 ); }
    if ( arg6.IsValid() ){ command.PushArg( arg6 ); }
    if ( arg7.IsValid() ){ command.PushArg( arg7 ); }
    if ( arg8.IsValid() ){ command.PushArg( arg8 ); }
    if ( arg9.IsValid() ){ command.PushArg( arg9 ); }

    LOG_DEBUG( "Calling Remote Function : " << pServerGameObjectId << "." << 
command.GetFunctionName() );
    ByteArray buffer( command.GetSerializeSize() );
    command.Serialize( buffer );

    ZMQMessage* request = new ZMQMessage();//deleted when sent via zmq
    request->addmem( buffer.GetBytes(), buffer.m_NumBytes );

    PushMessage( MDPW_REQUEST, m_pServiceId, request );
}
Danny Diaz
  • 230
  • 2
  • 10
  • Well, would you accept C variadics? :P – Rakete1111 May 13 '18 at 12:49
  • 2
    The problem is with the `virtual` there. C++ variadics (= variadic templates) are *template* based, which doesn't mix with `virtual`. – Angew is no longer proud of SO May 13 '18 at 12:51
  • 1
    sounds like you might want std::initializer_list – Killzone Kid May 13 '18 at 12:52
  • @Rakete1111, could you elaborate on how this could be done with C variadics? How to do without knowing the number of args? Are you referring to the macro magic that can be used to find the size? Note that this function is actually being called through macros. – Danny Diaz May 13 '18 at 13:09
  • @DannyDiaz Have a look at the example [here](http://en.cppreference.com/w/cpp/utility/variadic/va_start). – Rakete1111 May 13 '18 at 13:11
  • 2
    @DannyDiaz (Jftr and for future readers, don't *actually* do that C variadic stuff, though. Use the answer you accepted.) – Baum mit Augen May 13 '18 at 13:47
  • @Rakete1111 Class types that have a non-trivial copy c'tor, move c'tor or d'tor are _conditionally supported_ in varargs. Apart from that, they are of course lacking static type-safety or even support for dynamic type checking. I've fixed a number of bugs in other people's code where they passed `std::string` objects through `...` whereas `const char *` was expected on the other end, leading to UB. So you can guess I'm not a fan. I realize your suggestion is not entirely serious, of course. – Arne Vogel May 14 '18 at 09:55
  • @Arne yeah it wasn't. :) Didn't know this limitation, but it totally makes sense as the feature comes from C. Thanks! – Rakete1111 May 14 '18 at 13:53

2 Answers2

8

Sure, you can have that, if you want. You just have to combine a templated forwarder with a non-templated virtual function.

I'm using gsl::span ("What is a "span" and when should I use one?") and an automatic std::array (not a native one to avoid the special case of 0) for best generality, stability and efficiency.

virtual void DoCallRemoteFunction(
    const char* pServerGameObjectId,
    const char* pFunctionName,
    gsl::span<OVariant> args
) {
    ...
}

template<class... ARGS>
void CallRemoteFunction(
    const char* pServerGameObjectId,
    const char* pFunctionName,
    ARGS&&... args
) {
    std::array<OVariant, sizeof...(ARGS)> arr = { OVariant{std::forward<ARGS>(args)} ... };
    DoCallRemoteFunction(pServerGameObjectId, pFunctionName, arr);
}

I have given them different names because you presumably will override the virtual function, and probably don't want the API-function to be shadowed then.

Deduplicator
  • 44,692
  • 7
  • 66
  • 118
7

If you're willing to accept slightly different call syntax (which e.g. std::max also uses), you can have a very elegant solution:

virtual void CallRemoteFunction( const char* pServerGameObjectId, const char* pFunctionName, std::initializer_list<QVariant> args )
{
    std::vector<OVariant> argsArray;
    for (auto& arg : args )
    {
        argsArray.push_back(arg)
    }

    //do other stuff
}

Called like this:

CallRemoteFunction("foo", "bar", { arg1, arg2, arg3 });
Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • tx Angew for your quick response. Yes, I looked at doing it this way. But currently, I don't have to change my call syntax. As ugly as the function is, it works without any problems. Might as well just leave it as is, right? – Danny Diaz May 13 '18 at 13:07
  • @DannyDiaz See [Deduplicator's answer](https://stackoverflow.com/a/50316482/1782465) for how to wrap this to keep your current call syntax. – Angew is no longer proud of SO May 13 '18 at 13:24