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

C++: Generating similar methods with macros

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

Problem

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

#include
#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(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 \
(const char table, const char key, _type def) \
{

Solution

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
TypeOfLuaObject LuaState::lua;

template
struct DoCheck;
template
struct DoConversion;

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

Code Snippets

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;
}

Context

StackExchange Code Review Q#26291, answer score: 2

Revisions (0)

No revisions yet.