std::map, count values that are true

Question

I have a map:

std::map<std::string, bool> all_triggers_didfire;

I fill it and in the end would like to obtain the number of values that are true. The following code works:

int count_did_fire = std::count_if(
  all_triggers_didfire.begin(), 
  all_triggers_didfire.end(), 
  [](std::pair<std::string, bool> p){return p.second;}
);

Is there an easier way than to define a lambda expression for this?

Or `[](decltype(all_triggers)::value_type p) { return p.second; }`. — rodrigo, Apr 15 '15 at 10:29
The lambda seems fine to me, except that it should take its argument by reference to avoid memory allocation for copying the strings. (The same applies to the two comments above). — Jonathan Wakely, Apr 15 '15 at 13:16
@JonathanWakely and except that it needs to be `std::pair` to actually avoid copying :) — T.C., Apr 15 '15 at 23:49

score 11 · Answer 1 · edited Apr 15 '15 at 10:30

11

I would use std::set instead of std::map. They are semantically equivalent but using std::set is easier. Example:

std::set<std::string> triggers_that_did_fire;
int count_did_fire = triggers_that_did_fire.size();

When you originally populate the triggers_that_did_fire set, you can do the following:

triggers_that_did_fire.insert(mystring); //equivalent to setting to "true" in your map
triggers_that_did_fire.remove(mystring); //equivalent to setting to "false"

edited Apr 15 '15 at 10:30

IanPudney

5,941
1
24
39

answered Apr 15 '15 at 10:25

Alex Shtoff

2,520
1
25
53

2

@Steephen, I believe that the author has his/her own logical reasons to use map/set instead of std::vector. For example, maybe the author's software also wants to query to see if a specific trigger has indeed fired. – Alex Shtoff Apr 15 '15 at 10:32
Yes, std::set is more suitable if elements are added to the set repeatedly, or they can be removed after they are added, or both. – IanPudney Apr 15 '15 at 10:37
6

This is not an answer to the question. – Apr 15 '15 at 10:47
1

@Steephen: _"if std::set is possible here std::vector may be more suitable"_ Completely false – Lightness Races in Orbit Apr 15 '15 at 10:49
20

No, they are not semantically equivalent. A map of bools can map X to true, map X to false, or not map X at all. A set can only contain X or not contain X. – R. Martinho Fernandes Apr 15 '15 at 10:52
1

@Steephen: He _can_ but it would be a terrible approach. This is a perfect use case for sets. I don't know why you'd suggest switching to a less appropriate container. – Lightness Races in Orbit Apr 15 '15 at 11:08
@Steephen: You're not making much sense. If he "just needs to find the count" then he doesn't need a container at all; he already knows how many elements there are, because he put them there. He could just increment a counter and be done with it. No, he has a container that tells him _which_ triggers fired, and a set is the perfect container for that. It doesn't even matter that it's often implemented as a tree: it is a blob of unsorted, unique elements. It's for this. – Lightness Races in Orbit Apr 15 '15 at 11:36
@LightningRacisinObrit unsorted? or sorted? – Steephen Apr 15 '15 at 11:43
1

@Steephen: In practice they are of course exposed as sorted. Academically, though, I like to consider sets as blobs rather than sequences. They are, after all, associative containers _not_ sequence containers. You should not be using them for lists of things, but for collections of things. – Lightness Races in Orbit Apr 15 '15 at 12:22
1

@Steephen Not sorted in any way that you should rely on. – Taemyr Apr 15 '15 at 13:37
@LightningRacisinObrit, specifically in C++ std::set and std::map are ensured, by the standard, to be sorted. If you wish, you can rely on that fact. There is std::unordered_set and std::unordered_map for the "unordered blob" abstraction you talked about. – Alex Shtoff Apr 16 '15 at 08:04
1

In this case I am after a list of both triggers that fired and those that didn't. I could use two `set`s for that, but I am using a `map` because then I can easily print a sorted list of all triggers and print the "did fire" result for each. Therefore, while the answer is correct it doesn't answer the question because it requires a change in the code I would rather not make. – fuenfundachtzig Apr 16 '15 at 08:30
@Alex: You're conflating "ordered" and "sorted". I already conceded that there is an _apparent_ sorting due to the way the associative containers work, but this is essentially an abstraction leak. If you want a sequence of things, you use a sequence container. – Lightness Races in Orbit Apr 16 '15 at 10:34

Nim · Answer 2 · 2015-04-15T13:21:20.423

9

Sometimes, a simple for loop is a little clearer:

auto count = 0;
for (auto&& p : all_triggers_didfire)
  if (p.second)
    ++count;

EDIT 1: I'll post the original code in case anyone cannot see the edit history..

auto count = 0;
for (auto& p : all_triggers_didfire)
  count += p.second;

edited Apr 15 '15 at 13:21

answered Apr 15 '15 at 10:48

Nim

33,299
2
62
101

@LightningRacisinObrit, was that in relation to the transient comment that popped up - or something wrong? ;) – Nim Apr 15 '15 at 10:53
5

@rightfold, why the change? what's wrong with the way it was before? is it so horrible to rely on the bool<->int conversion? – Nim Apr 15 '15 at 10:55
2

Relying on such a conversion is confusing and makes the code less readable than an if statement. – Apr 15 '15 at 10:55
4

@rightfold, interesting, so you introduce an *universal reference* to "clarify" the code? :) Now when you look at the code above, what is the first thing that draws your attention? `&&` what is this thing and what is it doing there? IMO at this point one does a double-take and try to figure out what is going on there rather than skipping over the code that was there before as it was perfectly succinct... anyway, that's just me I guess... – Nim Apr 15 '15 at 11:08
15

@rightfold: I disagree with your edit -- it cuts across the poster's intentions. I see it as opinion-based. You should have commented, or posted our own answer. – TonyK Apr 15 '15 at 11:08
@Nim As an SO edit, I think it was inappropriate. But for the code itself, i depends on what you're used to, I guess. Forwarding references are perfectly normal to my eyes. – Angew is no longer proud of SO Apr 15 '15 at 11:11
@Agnew, for you, for me and for a bunch of folks kicking around on this site, but for some junior developer/c++11 beginner, who is picking up some code... different story I'd think... – Nim Apr 15 '15 at 11:17
3

@rightfold, using a forwarding reference here has **zero** benefit. We know that dereferencing a `map::iterator` produces an lvalue, so why bother making the code generic enough to handle rvalues that are impossible? – Jonathan Wakely Apr 15 '15 at 13:13
2

@JonathanWakely On the other hand, if `auto&&` can always be used whenever `auto&` can, you might legitimately read `auto&&` as "by reference", `auto&` as "by lvalue reference", and conclude that unless you specifically want to reject rvalues which you already know are impossible here, `auto&&` is the simple version to use when you don't want any added complexity. I don't entirely agree with that, but I do think it's a perfectly legitimate point of view, and that it's a simple matter of personal preference without strong technical advantages or disadvantages either way. – Apr 15 '15 at 13:49
2

@hvd, I agree there's an element of personal preference, but that also supports the comments above saying it's wrong to edit someone else's answer to reflect a personal preference that has no technical advantage – Jonathan Wakely Apr 15 '15 at 16:16
@JonathanWakely And I fully agree with those comments. The edit was just plain wrong, and I wouldn't dare claim otherwise. :) – Apr 15 '15 at 16:22

TartanLlama · Answer 3 · 2015-04-15T10:52:42.560

4

You could use std::mem_fn to wrap the access to the data member into a callable object:

int count_did_fire = std::count_if(
  all_triggers_didfire.begin(), 
  all_triggers_didfire.end(), 
  std::mem_fn(&decltype(all_triggers_didfire)::value_type::second)
);

edited Apr 15 '15 at 10:52

answered Apr 15 '15 at 10:46

TartanLlama

63,752
13
157
193

1

Any reason you're not using `std::mem_fn`? – Apr 15 '15 at 10:48

score 0 · Answer 4 · edited May 23 '17 at 12:15

Is there an easier way than to define a lambda expression for this? No.

Depends what you mean by easier. One important thing to remember here is that in C++, the value_type of a std::map is pair<const key_type,mapped_type> and not just the mapped_type. std::map::iterator iterates over this value_type and you need a wrapper to fetch either the key or the mapped type.

All C++ Standard Library algorithms work on iterators and for std::map its an iterator to value_type. So for the algorithms to work on the mapped type, we need to re-map the value_type to mapped type and for that either

You need a helper named function (pre C++11)
You would need a functor
Or, You would need a lambda.

It is worth noting that

"C++ Standard Library algorithms would be much more pleasant to use if C++ had support for lambdas"

A proposal to add lambda functions to the C++ standard, N1958=06-002.

So if you think your code looks ugly, your intention to cleanse the code would defeat the original motivation of lambda.

Thus, if you are intending to use C++ Standard Library algorithms, you need to use lambda if required as in the case of std::map (period). Of-course you can still re-write using iterative manner but it is a matter of choice and readability and "readability lies in the eyes of the reviewers"

std::map, count values that are true

4 Answers4