Skip to main content
Code Review

Return to Answer

added 1020 characters in body
Source Link
G. Sliepen
G. Sliepen
  • 61.7k
  • 3
  • 61
  • 152

Type alias for function pointers

In your code you have:

using function = action<Args...>; // action is a pointer for a function that returns 'void'

Instead of having a template of something and a comment to explain it, you can instead just write:

using function = void (*)(Args...);

Although it might be even better to name it function_pointer.

Use perfect forwarding where appropriate

If you have a bunch of parameters and want to pass them to another function, make sure you take the parameters by universal reference, and pass them on using std::forward:

void operator () (Args&&... args) {
    for (uint8_t i = 0; i < m_size; i++)
        m_functions[i](std::forward<Args>(args)...);
}

Sometimes that even makes sense for the constructor.

Alternatives

Ideally you just use a std::array of function pointers. It gets tricky in C++11 to have it deduce the right type from the constructor though; it's much easier in C++17 where you have class template argument deduction:

With C++20 you could use concepts to restrict Ts such that they are all the same and are function pointers, and that Args are valid for the type of function pointer, as this would give somewhat better error messages.

This works in C++11. However, although the class above requires C++17, it might still be perfectly usable on Arduino/ESP microcontrollers, assuming your toolchain has new enough compilers.

Ideally you just use a std::array of function pointers. It gets tricky in C++11 to have it deduce the right type from the constructor though; it's much easier in C++17 where you have class template argument deduction:

Type alias for function pointers

In your code you have:

using function = action<Args...>; // action is a pointer for a function that returns 'void'

Instead of having a template of something and a comment to explain it, you can instead just write:

using function = void (*)(Args...);

Although it might be even better to name it function_pointer.

Use perfect forwarding where appropriate

If you have a bunch of parameters and want to pass them to another function, make sure you take the parameters by universal reference, and pass them on using std::forward:

void operator () (Args&&... args) {
    for (uint8_t i = 0; i < m_size; i++)
        m_functions[i](std::forward<Args>(args)...);
}

Sometimes that even makes sense for the constructor.

Alternatives

Ideally you just use a std::array of function pointers. It gets tricky in C++11 to have it deduce the right type from the constructor though; it's much easier in C++17 where you have class template argument deduction:

With C++20 you could use concepts to restrict Ts such that they are all the same and are function pointers, and that Args are valid for the type of function pointer, as this would give somewhat better error messages.

This works in C++11. However, although the class above requires C++17, it might still be perfectly usable on Arduino/ESP microcontrollers, assuming your toolchain has new enough compilers.

Source Link
G. Sliepen
G. Sliepen
  • 61.7k
  • 3
  • 61
  • 152

Ideally you just use a std::array of function pointers. It gets tricky in C++11 to have it deduce the right type from the constructor though; it's much easier in C++17 where you have class template argument deduction:

template<typename... Ts>
class StaticFunctionGroup {
    using Function = std::common_type_t<Ts...>;
    std::array<Function, sizeof...(Ts)> functions;

public:
    constexpr StaticFunctionGroup(Ts&&... ts):
        functions{std::forward<Ts>(ts)...} {}

    template<typename... Args>
    void operator () (Args&&... args) {
        for (auto& function: functions)
            function(std::forward<Args>(args)...);
    }
};

And you can then write:

StaticFunctionGroup group{f1, f2,f3, f4};
group();

Much simpler though would be to create a stand-alone call_group() function that takes any container of function pointers as the first parameter:

template<typename T, typename... Args>
void call_group(const T& group, Args&&... args) {
    for (auto& function: group)
        function(std::forward<Args>(args)...);
}

And then write:

void (*group[])() = {f1, f2, f3, f4}; // but a suitable std::array works as well
call_group(group);