6
\$\begingroup\$

I am currently working on a project that involves Lua. For convenience, I created a wrapper class for the Lua state structure. However, I also added some methods for getting globals and table fields. Since these methods are very similar, I decided do turn them into macros.

Do you consider this a good solution? Would you have done it another way?

#include <lua/lua.hpp>

#include <util/Exception.h>
#include "LuaState.h"

namespace gnaar {

LuaState::LuaState() : ls(luaL_newstate())
{
    luaL_openlibs(ls);
}

void LuaState::doFile(const char* file)
{
    if (luaL_dofile(ls, file) != 0)
    {
        throwException(lua_tostring(ls, -1));
    }
}

lua_State* LuaState::operator*()
{
    return ls;
}

LuaState::~LuaState()
{
    lua_close(ls);
}

//==================== ugly macro stuff below, beware ====================

#define _getGlobal(_lua, _type, _check, _conversion)                    \
    template <>                                                         \
    _type LuaState::getGlobal<_type>(const char* name, _type dfault)    \
    {                                                                   \
        lua_getglobal(_lua, name);                                      \
        if (_check(_lua, -1))                                           \
        {                                                               \
            return (_type) _conversion(_lua, -1);                       \
        }                                                               \
        else                                                            \
        {                                                               \
            return dfault;                                              \
        }                                                               \
    }

#define _getTableField(_lua, _type, _check, _conversion)    \
    template <> _type LuaState::getTableField<_type>        \
    (const char* table, const char* key, _type def)         \
    {                                                       \
        lua_getglobal(_lua, table);                         \
        if (!lua_istable(_lua, -1))                         \
        {                                                   \
            return def;                                     \
        }                                                   \
        _type result;                                       \
        lua_pushstring(_lua, key);                          \
        lua_gettable(_lua, -2);                             \
        if (!_check(_lua, -1))                              \
        {                                                   \
            result = def;                                   \
        }                                                   \
        else                                                \
        {                                                   \
            result = (_type) _conversion(_lua, -1);         \
        }                                                   \
        lua_pop(_lua, 1);                                   \
        return result;                                      \
    }

_getGlobal(ls, int, lua_isnumber, lua_tonumber)

_getGlobal(ls, const char*, lua_isstring, lua_tostring)

_getGlobal(ls, bool, lua_isboolean, lua_toboolean)

_getTableField(ls, int, lua_isnumber, lua_tonumber)

_getTableField(ls, const char*, lua_isstring, lua_tostring)

_getTableField(ls, bool, lua_isboolean, lua_toboolean)

} // namespace gnaar
\$\endgroup\$
1
  • 1
    \$\begingroup\$ If you change (or wrap) functions like (lua_isnumber and lua_tonumber) or (lua_isstring lua_tostring) to function templates (e.g., lua_is<TypeHere> and lua_to<TypeHere>), you won't have to specialize getGlobal and getTableField manually/via macros and will be able to statically dispatch inside them automatically/via template-type selection. \$\endgroup\$ Commented May 17, 2013 at 17:49

1 Answer 1

Reset to default
2
\$\begingroup\$

Do you consider this a good solution?

No.

Would you have done it another way?

Yes.

Macros are never the solution. There is always a better tool. Macros are designed to deal with Hardware/OS/Compiler variations. You should limit their use to this situaation.

It looks like your macros are used to manually insert functions for specific situations. It seems like these could be deduced using template specializations. It is hard to provide a better solution without more code. But let my give it a go (this will probably be wrong as I don't have enough information).

template<typename Type>
TypeOfLuaObject LuaState::lua;

template<typename Type>
struct DoCheck;
template<typename Type>
struct DoConversion;

// Provide a specialization of these two macros for each
// type you are using.
template<>
struct DoCheck<int>
{
    bool operator()(TypeOfLuaObject& lauObjec, int val) const {return lua_isnumber(lauObjec, val);}
};
template<>
struct DoConversion<int>
{
    int operator()(TypeOfLuaObject& lauObjec, int val) const {return lua_tonumber(lauObjec, val);}
};

// The main function can then be left in its generalized form
// It will pick up the specialization of the above macros.
// Type will be deduced automatically from `def` parameter and the other two
// template parameters will be deduced from `Type` so you do not need to manually
// specify the type when calling.
template<typename Type, typename Check=DoCheck<Type>, Conversion=DoConversion<T>>
Type LuaState::getTableField(const char* table, const char* key, Type def)
{
    lua_getglobal(lua, table);
    if (!lua_istable(lua, -1))
    {   
        return def;
    }   
    Type result;
    lua_pushstring(lua, key);
    lua_gettable(lua, -2);
    Check doCheckObj;
    if (!checkObj(lua, -1))
    {   
        result = def;
    }   
    else
    {   
        Conversion doConversionObj;
        result = doConversionObj(lua, -1);
    }   
    lua_pop(lua, 1); 
    return result;
}

Other comments:

When you use macros. Make sure they are all uppercase.

Macros do not obey scope boundaries. So we reserve all uppercase letters for macros to prevent accidental smashing of normal variables.

Don't use '_' as a prefix for identifiers.
Most people don't know the actual rules for this. So even if you do it will confuse them.

But you are breaking the rule with your macros _getGlobal and _getTableField. As they are macros they are not bound by the scope of your namespace and thus could potentially be trampling on some system macro.

See the artile I wrote on SO: https://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier/228797#228797

\$\endgroup\$
2
  • \$\begingroup\$ Just one more question: Why are the the operators declared as 'const'? Sorry if that's a silly question, but I am relatively new to C++. \$\endgroup\$ Commented May 18, 2013 at 22:15
  • \$\begingroup\$ Methods declared const can not change the state of the object. Since there is no state held in the object that is not a problem. If an object is const you may only call const methods. So if you have a const object then you can only call methods that are const. As used in this example above it makes no difference. But when I add methods that do not change state I mark them as const. Then I can call them on a const object if I need too. It also provides context to the compiler allowing optimizations. \$\endgroup\$ Commented May 19, 2013 at 6:06

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.