Added make function for basic_bloom_filter

[amallia]

This is a version that uses universal reference. It is equivalent, but it will allow to save a copy of bitvector when it will become movable.

[mavam]

Ah, I just saw that one after commenting in #23. I'm closing this one as duplicate. The typical workflow would be to update the topic branch of your fork and (force-)push into it. This will update the github PR page and the review.

Feedback: you don't need a forwarding reference here. (The no-longer-common term coined by Scott Meyers was indeed universal reference, but we've moved on since.) The reason is that there's no use case where a const-reference (and thus copy) would make sense. Taking the bitvector by value was the right approach, which gives the caller the opportunity to decide how to pass in the instance.

[amallia]

I see your point. Copy-move or universalRef-forward have the same performance in any case, if I am not mistaken. Right?

On Fri, 23 Jun 2017 at 15:54, Matthias Vallentin notifications@github.com wrote:

Closed #24 https://github.com/mavam/libbf/pull/24.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mavam/libbf/pull/24#event-1136610461, or mute the thread https://github.com/notifications/unsubscribe-auth/AIo41IP0G9Foy1cTUvV9a9Rhb2kcRhT6ks5sG9GLgaJpZM4OCKI8 .

-- Sent from Gmail Mobile

[mavam]

Copy-move or universalRef-forward have the same performance in any case, if I am not mistaken. Right?

Right. In this case, there will always be a move inside the implementation, so there's no performance difference.

A good general design guideline is to use value semantics when possible. By making it a value, you can also move the implementation out of the header file and save compile cycles.

[amallia]

Thinking about it a bit better, to be honest, I realized that they are different in performance. In particular if you pass an rvalue to a value parameter it does an extra move that can be avoid with the universal reference. It is not a problem at all for me, but wanted to say that for the record.

[mavam]

Thinking about it a bit better, to be honest, I realized that they are different in performance.

I agree there's a difference in general, but let's look at the concrete use case here: you first deserialize the bit vector. Then you call the constructor f with f(std::move(bits)):

bitvector xs;
deserialize(xs);
construct(std::move(xs));

Here, it doesn't matter if the signature of construct is construct(bitvector) or construct(T&&), a sane compiler will generate the exact same code (and elide the extra move). Since this will be the most common usage scenario, I prefer the API with plain value semantics.