# C++11 factory pattern with lambdas

Anonymous functions are one of the best features of C++11. They make everything so beautiful!

However, one can get carried away and start overusing them.

This code calls a function that reads through a file and invokes a callback everytime that something matches a regex. Since I read two different files but I want callbacks only to differ for a constant, I chose this "lambda-returning-lambda" factory pattern.

auto callback_factory =
[&] (IPMarkerType start, IPMarkerType stop) -> std::function<void(IPNode<T>&)>
{
return [&,start,stop](IPNode<T> & node){
markers.push_back(IPMarker<T>(node.ip, start));
markers.push_back(IPMarker<T>(node.ip.network_ones(node.prefix), stop));
};
};



Callbacks are really local just to the calling function, so I think that external functor would not be justified. On the other hand, this code smells a lot to me, so I would like to hear your comments.

-
 Meh, looks good to me. Just make the node argument const&. – Konrad Rudolph Aug 13 '12 at 11:50 Any reason you need to declare the return type of the outer lambda? – ecatmur Aug 13 '12 at 11:55 @ecatmur Visual Studio 11 wouldn't compile otherwise. – Tibor Aug 13 '12 at 17:15

First of all, I will assume there is something like template <typename T> somewhere in your code. Otherwise, I doubt it would compile.

There is something you could do to improve your program: use emplace_back instead of push_back with an object construction. That way, objects will be created right in the collection (list, vector, etc...). By the way, I'll also assume that markers is a standard collection.

auto callback_factory =
[&] (IPMarkerType start, IPMarkerType stop) -> std::function<void(IPNode<T>&)>
{
return [&,start,stop](IPNode<T> & node){
markers.emplace_back(node.ip, start);
markers.emplace_back(node.ip.network_ones(node.prefix), stop);
};
};


And, yeah, something still smells: I really wonder how and/or where you declared markers. If it's a global variable, then I think the template parameter may cause some problems.

-
 I think it’s pretty obvious that this code is just part of a function, which obviates your first and last paragraph. – Konrad Rudolph Aug 13 '12 at 12:52 Beh, obvious things are not always that obvious for everyone. We can easily get lost. Anyway, the emplace_back stuff can still improve the function. – Morwenn Aug 13 '12 at 13:03

I suspect I'd hoist the start/stop parameters out, something like this:

template <typename T, typename Markers>
void regex_start_stop_wrapper(char const *path, char const *regex,
Markers &markers,
IPMarkerType start, IPMarkerType stop)
{
[&markers,start,stop](IPNode<T> & node)
{
markers.push_back(IPMarker<T>(node.ip, start));
markers.push_back(IPMarker<T>(node.ip.network_ones(node.prefix), stop));
}
);
}

void test()
{
// ... original call sites now look like this:
regex_start_stop_wrapper<T>("C:\\path\\to\\file1.txt", regex,
markers, ipm_a_open, ipm_a_close);
regex_start_stop_wrapper<T>("C:\\path\\to\\file2.txt", regex,
markers, ipm_b_open, ipm_b_close);
}


It just seems like a simpler interaction between the callback, the call site and the read_regexp, somehow. I think it's because this is a straighter call chain, compared to the factory-lambda-returning-a-nested-lambda approach, which is a bit like the world's densest dependency injection.

-