4
\$\begingroup\$

This is a class which stores a function pointer and arguments to that function, to call/invoke that function at a later point in time, or multiple times. Additionally, through polymorphism, it allows methods with the same return type (though potentially different arguments) to be grouped (in a container) and invoked being agnostic to the actual function.

The main areas I'm interested in are correctness (use of const-correctness, move semantics, etc.), ease of use and performance.

The code:

#pragma once

#include <tuple>
#include <cstdint>

/**
 * @brief  Contains internal details for implementing FunctionInvocation.
 */
namespace detail
{
    /**
     * @brief  Structure whose template arguments represent a sequence
     *         of numbers. Use @ref SequenceGen to generate such a type.
     */
    template<::std::size_t... TNumbers>
    struct Sequence { };

    /**
     * @brief  When given a single template parameter it'll generate a
     *         @ref Sequence for which the TNumbers is a sequence from
     *         0 to TNumber (inclusive).
     *
     * @param  TNumber  The number towards which the sequence should
     *                  count (inclusive).
     *
     * @remark You cannot pass in a negative number.
     */
    template<::std::size_t... TNumber>
    struct SequenceGen;

    template<::std::size_t... TSequence>
    struct SequenceGen<0, TSequence...>
    {
        /**
         * @brief  The type of the sequence.
         *
         * Will contain a sequence from 0 to TNumber (inclusive).
         * For example, SequenceGen<2>::Type will return Sequence<0, 1, 2>.
         */
        using Type = Sequence<0, TSequence...>;
    };

    template<::std::size_t TCurrentNumber, ::std::size_t... TSequence>
    struct SequenceGen<TCurrentNumber, TSequence...> : SequenceGen<TCurrentNumber - 1, TCurrentNumber - 0, TSequence...>
    {
        static_assert(TCurrentNumber > 0, "Sequence Generator can only take positive integers.");
    };

    /**
     * @brief  Helper function to invoke a method with arguments supplied through a tuple.
     */
    template<typename TReturn, typename... TArgs, ::std::size_t... TNumbers>
    inline TReturn InvocationHelper(TReturn(*function)(TArgs...), const std::tuple<TArgs...>& arguments, Sequence<TNumbers...>)
    {
        // Invoke the function with a copy of all items in the tuple.
        return function(TArgs(std::get<TNumbers>(arguments))...);
    }
}

/**
 * @brief  Abstract base class for invoking functions from which
 *         the parameters are generated or defined by a subclass.
 *
 * @param  TReturn  The return value of the function including all
 *                  of its modifiers (ie. pointer, reference, const).
 */
template<typename TReturn>
class FunctionInvocation abstract
{
public:

    /**
     * @brief  Invokes the function.
     *
     * @remark Derived classes may set specific constraints regarding
     *         preconditions and performance of invoking the function.
     *         Consult their documentation for more information.
     *
     * @return The return value of the invoked function.
     */
    virtual TReturn operator()() const = 0;
    virtual ~FunctionInvocation() { }

};

/**
 * @brief  A specific type of @ref FunctionInvocation that stores
 *         a function pointer and a tuple of arguments to be invoked
 *         at a later time or multiple times.
 *
 * @param  TReturn  The return value of the function including all
 *                  of its modifiers (ie. pointer, reference, const).
 * @param  TArgs    The types of arguments to pass to the function,
 *                  including all of its modifiers.
 */
template<typename TReturn, typename... TArgs>
class StoredFunctionInvocation final : public FunctionInvocation<TReturn>
{
public:
    using Delegate = TReturn(*)(TArgs...);

private:
    ::std::tuple<TArgs...> _args;
    Delegate _fn;

public:

    /**
     * @brief  Default constructor. Will result in an empty invocation object.
     */
    inline StoredFunctionInvocation()
        : _fn  (nullptr),
          _args() { }

    /**
     * @brief  Creates an invocation object with a function and a set of arguments.
     *
     * @param  function   The function to call upon invocation.
     * @param  arguments  The arguments to pass to the function.
     *
     * @remark The constructor does not call the function.
     */
    inline StoredFunctionInvocation(Delegate function, TArgs&&... arguments)
        : _fn  (function),
          _args(std::forward<TArgs>(arguments)...) { }

    /**
     * @brief  Copies an invocation object, including all the arguments.
     *
     * @param  other  The object to copy into this new one.
     */
    inline StoredFunctionInvocation(const StoredFunctionInvocation& other)
        : _fn  (other._fn),
          _args(other._args) { }

    /**
     * @brief  Assign an invocation object, including all the arguments.
     *
     * @param  other  The object whose values to assign into this object.
     *
     * @remark Self-assignment will leave the object unmodified.
     */
    inline StoredFunctionInvocation& operator=(const StoredFunctionInvocation& other)
    {
        // Prevent self assignment
        if (this == &other) return *this;

        _fn   = other._fn;
        _args = other._args;

        return *this;
    }

    /**
     * @brief  Moves an invocation object, emptying the other object.
     *
     * @param  other  The object to move into this new one.
     */
    StoredFunctionInvocation(StoredFunctionInvocation&& other)
        : _fn  (other._fn),
          _args(std::move(other._args))
    {
        other._fn = nullptr; // Empty the other object.
    }

    /**
     * @brief  Moves an invocation object into the current object,
     *         emptying the other object.
     *
     * @param  other  The whose values to move into this one.
     *
     * @remark Self-assignment will leave the object unmodified.
     */
    StoredFunctionInvocation& operator=(StoredFunctionInvocation&& other)
    {
        if (this == &other) return *this;

        _fn   = other._fn;
        _args = std::move(other._fn);

        other._fn = nullptr;

        return *this;
    }

    /**
     * @brief  Tests whether this object is empty.
     *
     * @return True if empty, otherwise false.
     */
    inline bool IsEmpty() const { return _fn == nullptr; }

    /**
     * @brief  Invokes the function with its arguments.
     *
     * @remark Calling it on an empty object is undefined behavior.
     * @reamrk Invoking the function will copy all arguments once.
     *
     * @return The return value of the invoked function.
     */
    inline virtual TReturn operator()() const override
    {
        return detail::InvocationHelper<TReturn, TArgs...>(_fn, _args, typename detail::SequenceGen<sizeof...(TArgs) - 1>::Type());
    }

};

Example usage code, no need for reviewing this, just provided for reference:

#include <iostream>
#include "FunctionInvocation.h"

int fn1(int arg1, int arg2)
{
    std::cout << "Fn1: " << arg1 << ", " << arg2 << std::endl;
    return arg1 + arg2;
}

int fn2(int arg1, const int& arg2)
{
    std::cout << "Fn2: " << arg1 << ", " << arg2 << std::endl;
    return arg1 * arg2;
}

int fn3(int arg1, int arg2, int arg3)
{
    std::cout << "Fn3: " << arg1 << ", " << arg2 << ", " << arg3 << std::endl;
    return arg1 + arg2 - arg3;
}

void fn4(int arg1)
{
    std::cout << "fn4: " << arg1 << std::endl;
}

int main(int argc, char** argv)
{
    StoredFunctionInvocation<int, int, int>        invoc1(fn1, 1, 2);
    StoredFunctionInvocation<int, int, const int&> invoc2(fn2, 3, 4);
    StoredFunctionInvocation<int, int, int>        invoc3(fn1, 5, 6);
    StoredFunctionInvocation<int, int, int>        invoc4(fn1, 7, 8);
    StoredFunctionInvocation<int, int, int, int>   invoc5(fn3, 3, 2, 1);
    StoredFunctionInvocation<int, int, int, int>   invoc6(fn3, 6, 5, 4);

    // Test with a void return type
    StoredFunctionInvocation<void, int>            invoc7(fn4, 10);

    FunctionInvocation<int>* ginvoc[6] =
    { &invoc1, &invoc2, &invoc3, &invoc4, &invoc5, &invoc6 };

    for (std::uint32_t i = 0; i < 6; i++)
    {
        std::cout << "Result: " << (*ginvoc[i])() << std::endl;
    }

    invoc7();

    std::cin.get();

    return 0;
}
\$\endgroup\$
  • \$\begingroup\$ Why not just use std::bind? \$\endgroup\$ – Barry Oct 23 '15 at 20:27
  • \$\begingroup\$ @Barry a few reasons really, one is because I wanted to practice templates and variadic templates, another is because of the lack of control over it. I wanted a strong guarantee that the object would always be completely self-contained and require no memory allocations. That's not for the sake of performance, but because of incorporation with custom memory management strategies. \$\endgroup\$ – Aidiakapi Oct 23 '15 at 20:54
4
\$\begingroup\$

Reinventing the wheel

What you're writing is basically a strictly worse version of bind. It's strictly worse for two reasons. First, it only allows for function pointers. You can't use member functions or callable objects. Second, you can't partially apply functions. It's all or nothing. So, it's not nearly as useful as it could be. If the goal of this exercise is simply to reimplement bind, it's definitely worth trying to support both of those features.

Rule of Zero

Your class has two members: A std::tuple and a function pointer. These are already copyable and movable where appropriate. As such, writing your own operations is at best, a waste of code and at worst, error-prone.

You'll need to default the constructor:

std::tuple<TArgs...> _args;
Delegate _fn = nullptr;
StoredFunctionInvocation() = default;

But then for the other special member functions, you should either default all of them or omit all of them:

~StoredFunctionInvocation() = default;
StoredFunctionInvocation(const StoredFunctionInvocation&) = default;
StoredFunctionInvocation(StoredFunctionInvocation&& ) = default;
StoredFunctionInvocation& operator=(const StoredFunctionInvocation&) = default;
StoredFunctionInvocation& operator=(StoredFunctionInvocation&&) = default;

Prefer omission.

Construction

You are allowing your arguments to only be constructed by rvalue reference. TArgs&&... is not a forwarding reference since TArgs is not a template argument of the function. As such, the forward() call is really just a move(). Prefer:

template <typename... Us,
          typename = std::enable_if_t<std::is_constructible<
              std::tuple<TArgs...>(Us&&...)>::value>
          >
StoredFunctionInvocation(Delegate function, Us&&... args)
: _fn(function)
, _args(std::forward<Us>(args)...)
{ }

The inline is unnecessary.

IsEmpty

This function should be named explicit operator bool() const for consistency with other similar container-like objects.

Template Signature

You are currently just listing all your types in order. However, if you look at all the other class templates that take a callable, they always take a single template argument as the signature. This makes the usage more explicit. That is:

StoredFunction<int(int, int)> invoc1(fn1, 1, 2);

Polymorphism

Having virtual polymorphism isn't useful here, since then the user has to keep track of allocated memory. I'd want to be able to write something like:

FunctionInvocation<int()> invoc1 = StoredFunction<int(int, int)>(fn1, 1, 2);

And have that handle the type erasure. I don't want to have to write:

FunctionInvocation<int()>* invoc1 = new StoredFunction<int(int, int)>(fn1, 1, 2);

since then I'd have to write (*invoc1)(), which is quite unnatural. While we're add it, I don't want to have to spell out StoredFunction<int(int, int)>(fn1, 1, 2) either and would much rather be able to write:

FunctionInvocation<int()> invoc1 = StoreFunction(fn1, 1, 2);

which, going back to my initial point, is exactly what I would have to write with the standard library:

std::function<int()> invoc1 = std::bind(fn1, 1, 2);

Sequence

SequenceGen<N> should generate the sequence <0, 1, 2, ..., N-1>. That will make it much more useful. Otherwise, you will end up calling SequenceGen<X - 1> a lot.

Also, you should provide an alias template from SequenceGen, so that you can simply write:

return InvocationHelper(_fn, _args, make_sequence<sizeof...(TArgs)>{});

You should not need to specify the template arguments here as they should be deducible from _fn and _args.

InvocationHelper

Why are you invoking the function with a copy of all of the items? You're already copying them once (into the tuple). Why copy them again?

Comment Style

It's great that you're writing lots of comments, but this is a truly excessive amount of comments. There is an enormous amount of grey text here, and it actually didn't help me understand anything hardly at all. Do you need to write a 5-line comment on what the copy constructor does? It's a copy constructor. Do you need an 8-line comment on move assignment? It's move assignment.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks for the many points of improvement. Many things I can take away and learn from this. Just to make sure I didn't make an oversight. Upon construction the arguments can be moved or copied into the actual container object, but upon invocation, they should be copied. Because the function can be invoked multiple times, and I wouldn't want to move and invalidate the arguments, because that'd then pass invalid arguments upon a second invocation. Am I missing something here? \$\endgroup\$ – Aidiakapi Oct 23 '15 at 21:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.