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
1 Answer 1
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
-
\$\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\$Tobyte M.D.– Tobyte M.D.2013年05月18日 22:15:26 +00:00Commented 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\$Loki Astari– Loki Astari2013年05月19日 06:06:56 +00:00Commented May 19, 2013 at 6:06
lua_isnumber
andlua_tonumber
) or (lua_isstring
lua_tostring
) to function templates (e.g.,lua_is<TypeHere>
andlua_to<TypeHere>
), you won't have to specializegetGlobal
andgetTableField
manually/via macros and will be able to statically dispatch inside them automatically/via template-type selection. \$\endgroup\$