Function wrapper that prints the result

Question

(English) I was recently challenged to make a simple program that does the following:

Function f accepts as a parameter a function g, which returns a value. Function f must return a function h which returns and prints the value that function g returns.

After some research I got the solution and the program was successful, but I wonder if the code is done in the best way. I would appreciate immensely a critique.

This is the code:

Hace poco me retaron a realizar un programa sencillo que realiza lo siguiente:

La función f acepta como parámetro una función g, la cual regresa un valor. La función f debe de regresar una función h la cual regresa e imprime el valor que regresa la función g.

Luego de investigar un poco conseguí la solución y el programa resulto exitoso, pero me pregunto si el código esta hecho de la mejor manera. Agradecería inmensamente una critica.

Este es el código:

#include <iostream>
using namespace std;

auto g() // Función que regresara el valor principal
{
    return 123;
}

template <class FT>

auto f(FT* fref) // Función que recibira una referencia de una función
{
    return [=]() -> auto // Regresa una función creada a partir de "fref"
    {
        cout << fref() << endl;
        return fref();
    };
}

int main()
{
    auto h = f(&g); // Llamando la función f pasando la referencia de la función g
    auto resultado = h();
    cout << resultado << endl;
    return 0;
}

Answer 1

I see one substantial problem with your implementation:

f() calls fref() twice - bad enough if each call consumes considerable resources,
probably plain wrong where the call has side effects.

(The funny part is main() introducing a variable to hold the result without need.)

Answer 2

using namespace std; is a harmful habit you'd do well to shake off. It's less effort to write std:: in the four places you use it (two if you stop flushing with every newline) anyway.

Instead of passing a pointer to your function, it's probably simpler to pass the invocable itself. That allows g to be something other than a plain function - perhaps a std::function or some other object with operator(). It's quite reasonable for the template argument to deduce to be a pointer type, so that won't affect the current usage. A function name decays to a pointer to the function, so g would be as good as &g anyway.

Instead of a catch-all capture, I think it's better to be specific: replace [=] with [fref].

The -> auto return type will always return by value, but if the wrapped function returns a reference, we should be returning the same reference, rather than a copy of the referent. That would be -> decltype(fref(declval(args)...)) or more simply, -> decltype(auto).

It seems unnecessary for main() to also print the result, when that's what the function itself does.

Simplified thus, we get:

template<class F>
auto f(F fref)
{
    return [fref] -> decltype(auto) {
        auto&& result = fref();
        std::cout << result << '\n';
        return result;
    };
}

We should probably constrain F to be something we can invoke:

#include <concepts>

template<typename F>
auto f(F fref)
    requires std::invocable<F>
{
    return [fref] -> decltype(auto) {
        auto&& result = fref();
        std::cout << result << '\n';
        return result;
    };
}

Perhaps we should be passing the output stream as argument to h(), given the problem statement didn't say which stream to print to.

What about functions that take arguments? It wasn't specified that g() would be a function of no arguments. Let's pass arguments through, and supply the stream at call time:

#include <concepts>
#include <ostream>
#include <utility>
    
template<typename F>
auto f(F fref)
{
    return [fref](std::ostream& os, auto&&... args)
        -> decltype(auto)
    {
        auto&& result = fref(std::forward<decltype(args)>(args)...);
        os << result << '\n';
        return std::forward<decltype(result)>(result);
    };
}

int main()
{
    auto const h = f(g);
    h(std::cout);
}