HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Call a lua function - the C++ way

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thefunctionwaycalllua

Problem

I'm not that good when it comes to templates, especially when it comes to variadic templates. I want to show you some code where I use both - templates and variadic templates - to call a lua function. I'm proud that it works, and it works great, but I want to get some feedback on what I could've done better.

template
bool run_script(int id, Args && ... args) {
    const int params = sizeof...(args);
    _load(id);
    push(std::forward(args)...);
    return _run(params);
}


This is the "entry point". _load will just load the lua-function onto the stack and _run will run it with params arguments. The important part is the push function.

template
void push(T && arg) {
    _push(std::forward(arg));
}

template
void push(T && arg, Args && ... other) {
    _push(std::forward(arg));
    push(std::forward(other)...);
}

void _push(int arg) {
    lua_pushinteger(L, arg);
}

void _push(const std::string& arg) {
    lua_pushstring(L, arg.c_str());
}

void _push(const char* arg) {
    lua_pushstring(L, arg);
}


I had a problem before, where I had all of them named push, but for some reason it couldn't push a std::string (std::string as argument choose the template instead of const std::string&) but now with the templates called push and the functions which do the work _push it's working great. It's not much code, I know, but it's my the first time working with variadic templates and it took me a fair amount of time to get it working.

Solution

As your code goes everything i believe is fine. Define the template, define overloads and combine. Some things i've noticed that might be arguable:

-
Is it possible that your script will have zero arguments? For example a script with id = N denotes print last command or show commands history. If yes, your code wont compile since

template   
void push(T && arg) {
   _push(std::forward(arg));
}


doesnt accept zero arguments. Perhaps replace it with zero arg overload?

template
void push() {
    // cout << "All parameters passed to Lua! [Empty args case]\n";  
}


-
Also as Args && ... other can actually be zero length there's a question of which will be called template void push(T && arg) or template void push(T && arg, Args && ... other). Since push(std::forward(other)...) can not compile with 0 args, then the first overload will get called. I'd call that a bit misleading if somebody else has to work with your code.

-
The run_script func is packed with expressions which could potentially fail. So i'd probably try to encapsulate each step into its own method and move the _run() to the zero template push.

template
void push() {
    // cout << "All parameters passed to Lua! [Empty args case]\n";
    _run(params);     
}


That way if push() gets called you're sure all the parameters were passed to interpreter.

-
Question arises here is how do you pass params to zero arg push? The topic says C++ way and that means OOP. Imo if you're asking about the C++ style then perhaps you should encapsulate your logic and provide an interface to the user. So go for a class which would hold the arg number as a private member and handle the actual processing of different types. Perhaps two classes one for command and one for processing it. That way it will be easier to scale.

And before doing that i'd think about the actual use of your application:

  • Will somebody else will be using it?



  • Will it be used in a multithreaded environment?



  • What guarantees for the user do you want to provide?



-
Your entry function bool run_script is supposed to return true or false i guess depending on how the execution of the script went. Does it really boil down to two outcomes? Are you giving a guarantee that the code would execute no matter what? Perhaps a enum class would suit better.

-
Your lua_pushstring(L, arg) and lua_pushinteger(L, arg) all hold some L - so seems only fair to hide it somewhere in the class as a private member as well as arg num.

-
Are you sure that you'll know all your types at compile time? What if somebody passes a double? There are more elegant ways to check the type being passed but this will do:

template 
struct eq {
   static const bool result = false;
};

template struct eq {
    static const bool result = true;
};


and use

if (!eq::result) {
    //...unsupported type error
}


p.s. great project btw. A while a go I've used sockets in Lua and C++ to communicate. So if your code is online I'd love to check it out.

Code Snippets

template<typename T>   
void push(T && arg) {
   _push(std::forward<T>(arg));
}
template<typename T>
void push() {
    // cout << "All parameters passed to Lua! [Empty args case]\n";  
}
template<typename T>
void push() {
    // cout << "All parameters passed to Lua! [Empty args case]\n";
    _run(params);     
}
template <class A, class B>
struct eq {
   static const bool result = false;
};

template<class A> struct eq<A, A> {
    static const bool result = true;
};
if (!eq<int, A>::result) {
    //...unsupported type error
}

Context

StackExchange Code Review Q#139948, answer score: 4

Revisions (0)

No revisions yet.