C++11 factory pattern with lambdas

Question

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));
 };
};
read_regexp<T>("C:\\path\\to\\file1.txt", regex, callback_factory(ipm_a_open,ipm_a_close));
read_regexp<T>("C:\\path\\to\\file2.txt", regex, callback_factory(ipm_b_open,ipm_b_close));

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.

• Konrad Rudolph: Meh, looks good to me. Just make the node argument const&.
• ecatmur: Any reason you need to declare the return type of the outer lambda?
• user13049: @ecatmur Visual Studio 11 wouldn't compile otherwise.

Answer 1

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.

• Konrad Rudolph: I think it’s pretty obvious that this code is just part of a function, which obviates your first and last paragraph.

Answer 2

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)
{
 read_regexp<T>(path, regex,
 [&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.