Storing and loading functions

Question

The class Functions can store/load any functions. It does this by assigning a unique name to every function that gets added. You can then call the function by specifying its name and return type.

Here is a sample usage

//Create class
Functions funcs;

//Add 2 functions (foo -> void(int, int) | add -> int(int, int))
funcs.add("foo", foo);
funcs.add("add", add);

//Call them
int r = funcs.call<int>("add", 3, 2);
funcs.call<void>("foo", r, 2);

Here is the actual class

class Functions
{
public:
    template<typename T, typename... Ts>
    void add(std::string name, T(*func)(Ts...))
    {
        mFunctions.insert(std::make_pair(name, func));
    }

    template<typename T, typename... Ts>
    T call(std::string name, Ts... args)
    {
        try
        {
            return boost::any_cast<T(*)(Ts...)>(mFunctions.at(name))(args...);
        }
        catch (const std::out_of_range&)
        {
            throw std::invalid_argument("Function doesn't exist!");
        }
    }
private:
    std::map<std::string, boost::any> mFunctions;
};

Is there anything to be improved, like maybe naming, programming techniques, ...? Some general feedback would be appreciated.

Answer 1

You'll probably want to use perfect forwarding in the call method to preserve the type qualifiers. Right now the arguments are always copied, which might be inefficient for more complex user defined types.

template<typename RetType, typename... Args>
RetType call(const std::string& name, Args&&... args) const
{
    // ... lookup ...
    return func(std::forward<Args>(args)...);
}

Also note that the function name can be taken by const ref, since you just need to look it up. No additional copies are required.

It also seems adequate to const qualify the method, since it will not alter the underlaying map container.

Speaking of std::map, I would avoid it in favor of the newer std::unordered_map. std::map is a BTree, so it is guaranteed to allocate memory at each insertion for its internal tree nodes, making it memory and cache inefficient. unordered_map is a hash table, so it should have better average lookup time and allocate memory less often. I suggest using it as the new default for key+value store unless you are certain map is more efficient in your usage case.

   try
   {
       return boost::any_cast<T(*)(Ts...)>(mFunctions.at(name))(args...);
   }
   catch (const std::out_of_range&)
   {
       throw std::invalid_argument("Function doesn't exist!");
   }

This is not incorrect or bad code, but map and unordered_map also have lookup methods that don't throw. Code that doesn't generate exceptions tend to be more efficient and might even convey intent more clearly. Alternatively, using the find method:

const auto iter = mFunctions.find(name);
if (iter == std::end(mFunctions))
{
    // throw
}

const auto func = boost::any_cast<T(*)(Args&&...)>(iter.second);

Answer 2

Its a good start.
But function pointers are not the only type callable objects. Functors were a big part of C++03 and with C++11 lambdas are even more important.

Rather than using:

T(*func)(Ts...)

Which handles function pointers. You can generalize this with std::function<>.

std::function<T(Ts...)>

Which generalizes the callable construct.