binding function with external state to member with no knowledge of said state

[orchid trellis]

Assuming I have a class that requires a custom comparator that is not known at compile time,

template <class T>
using Comparator = std::function<int(const T&, const T&)>;

class Foo {
public:
  Foo(Comparator<T> cmp)
    : cmp_{cmp}
  {}
private:
  Comparator cmp_;
};

and I want to allow for a more generalized comparator that allows for external state,

template <class T>
using uComparator = std::function<int(const T&, const T&, void *)>;

which of these options will lead to the least headaches in the future? (and do they even do what I expect them to do?)

template <class T>
Foo<T>::Foo(uComparator<T> ucmp, void *udata)
  : cmp_{[ucmp, udata](const T& a, const T& b){ return ucmp(a, b, udata); }}
{}
template <class T>
Foo<T>::Foo(uComparator<T> ucmp, void *udata)
  : cmp_{std::bind(ucmp, _1, _2, udata)}
{}

is there a better way to be going about this?

[trim sandal]

When your question is answered use !solved to mark the question as resolved.

Remember to ask specific questions, provide necessary details, and reduce your question to its simplest form. For tips on how to ask a good question run !howto ask.

[worldly skiff]

both sound terrible to me, the external state really looks like it should be an implementation detail, and Foo sounds like it should only get a comparator with a comparator interface and let the caller/builder deal with this, in terms of long-term maintenance and separation of concerns that's what I'd consider simplest

if the case is common enough, having some helpers to let you set things up quickly is fine, but I still wouldn't put that into Foo's interface or the requirements for the comparator

especially as it is highly dubious how the "external state"'s lifetime should be handled or if it even is fine to just keep a pointer to it around

say you have a C-function with external state and that its interface happens to be void ccompare(state_t *state, T const *lhs, T const *rhs); you'd kinda have to setup a whole new dedicated interface for it or just reuse the existing one

in terms of existing interface, the comparator is more than enough, if you kick aside lifetime of the external state, the caller you can just pass the right one-liner lambda into the comparator interface

[&state] (const T& a, const T& b) { return ccompare(&state, &a, &b); }

if the state's lifetime actually needs to be handled properly, you'd make an actual function object type

as for the things you've suggested, I guess I'd lean more towards the lambda, though arguably I'd move the argument ucmp to the lambda's captures

actually same for the bind case, you'd move the thing

anyway both should mostly do what you want, just that bind feels not quite adequate here, as the things that bind uniquely does (that lambdas do not do) don't come into play here

and I'd personally prefer taking the "ucomparator" as a deduced, constrained template parameter, rather than force a conversion to a std::function

[orchid trellis]

ok, thanks.

tbh I agree with the decision to keep with only one comparator type and to have the user deal with it as you said, a single oneliner lambda.

but i am also the user, Foo is completely internal, and I am guaranteed that udata lives as long as Foo, in fact longer.

not to say that relying on that is a good idea, but my boss is very wary of diverging from the C code this is based off of. in that case, the struct contained only int (*cmp)(const void *, const void *, void *) and also saved the pointer to the external sate void *udata, which in the case of a basic comparator that had no such requirement, was simply NULL.

i'm trying to wean this off to slightly more idiomatic C++, one small step at a time

[trim sandal]

@orchid trellis Has your question been resolved? If so, run !solved :)

[orchid trellis]

!solved