CODEWITHSUNDEEP
c++lambda
black sheep
black sheep

Reputation: 399

Replacing a function with a lambda in C++

I have a function that just sets a value of 1 to a struct member variable length. In modern C++ this doesn't seem like good code style. Could this be done with a lambda? 

void setEdgeLengths(Koala::AssocArray <koalaGraph::PEdge, Koala::DijkstraHeap::EdgeLabs<int >> &edgeMap, std::vector<koalaGraph::PEdge>& E) 
{
    for (size_t i = 0; i < E.size(); i++) {
    edgeMap[E[i]].length = 1;
    }
}

The reason I am asking is https://shaharmike.com/cpp/lambdas-and-functions/ suggests that the lambda will be faster than an ordinary function.

Lambdas are also awesome when it comes to performance. Because they are objects rather than pointers they can be inlined very easily by the compiler, much like functors. This means that calling a lambda many times (such as with std::sort or std::copy_if) is much better than using a global function. This is one example of where C++ is actually faster than C.

Upvotes: 0

Views: 1493

Answers (2)

Max Langhof
Max Langhof

Reputation: 23691

I would consider the following code optimal (aside from given variable names):

void setEdgeLengths(Koala::AssocArray <koalaGraph::PEdge, Koala::DijkstraHeap::EdgeLabs<int >> &edgeMap, std::vector<koalaGraph::PEdge>& E) 
{
    for (const auto& e : E) {
        edgeMap[e].length = 1;
    }
}

Style (or omit) the curly braces as you like.

You can put any or all of this into arbitrarily many nested lambdas, but this is no more useful (but likely more harmful, at least in debug builds) than adding more whitespace. What you probably wanted to ask for is this:

void setEdgeLengths(Koala::AssocArray <koalaGraph::PEdge, Koala::DijkstraHeap::EdgeLabs<int >> &edgeMap, std::vector<koalaGraph::PEdge>& E) 
{
    std::for_each(E.begin(), E.end(), [&edgeMap](const auto& e) {
        edgeMap[e].length = 1;
    });
}

You no longer have a plain loop (which some advocate as good style), but I don't think the code has become any clearer. It also didn't get faster from doing this - if anything, debug performance is likely reduced slightly.

Now, the latter form does allow for parallel execution by doing

std::for_each(std::execution::parallel, E.begin(), E.end(), [&edgeMap](const auto& e) {

but this is only legal if your edgeMap properly handles concurrent access. If it was a std::map, operator[] could potentially insert a new element (which is not thread-safe) so without further assumptions this would not be a legal optimization.

Upvotes: 3

NathanOliver
NathanOliver

Reputation: 180595

If you want to apply an operation to every element of a container then you can use std::for_each. Since you want to use every element of E to access edgeMap you'll use for_each on E like

Koala::AssocArray <koalaGraph::PEdge, Koala::DijkstraHeap::EdgeLabs<int >> edgeMap = /* stuff */;
std::vector<koalaGraph::PEdge> E = /* stuff */;

std::for_each(E.begin(), E.end(),[&](auto const& index){ edgeMap[index].length = 1; });

Upvotes: 1

Related Questions