Readable alternative to std::set::find

Question

Usually I don't care about the value of the item in the set, I care only whether or not it exists. And repeatedly writing if (s.find(val) == s.end()) is long, ugly and less readable.

Is there a nice way which looks like if (contains(s,val)), which is somewhat standard (stl, boost).

Bonus point for a solution that works for maps as well.

Yes, I know I can

#define has(X,Y) (X).find(Y) != (X).end()
template<T,U> inline bool has(T s,U elt) {return s.find(elt) != s.end();}

but I'd rather use a best practice than reinventing a lot of small utilities.

Duplicate of http://stackoverflow.com/questions/1701067/stl-how-to-check-that-an-element-is-in-a-stdset, Adhemar has an interesting take on why stl containers don't include a contains() — Patrick, Mar 15 '11 at 11:29
Why is it ugly? Any C++ programmer can read and understand it. That alone makes it good practice, no matter someone's personal, religious believes about coding style. Why are you concerned of whether a non-C++programming person can understand your C++ code or not? — Lundin, Mar 16 '11 at 11:55
@Lundin, C++ programmer ≠ STL programmer. And anyhow English (`box.contains(item)`) is more common than STL idioms. Last and most important thing, it is very long and repeats the name of the container twice which gives you another place to err (`u.find(x) != v.end()`). — Elazar Leibovich, Mar 28 '11 at 11:44
@Elazar STL has been part of C++ since when, 1995? Apart from that, some of the most horrid programs I have ever seen was written by people trying to reinvent a programming language with their own macros. It is pretty much the same thing as saying "I don't like the English word _programming_, it is such a long word and isn't really a suitable description for what I do. Henceforth I will use the word _encoding_ instead, as it clearly suits better. If nobody understands wtf I'm saying no more, they should learn my own correct version of English instead." — Lundin, Mar 28 '11 at 13:58
@Lundin, as I said, C++ is not STL. There are programmers who know Qt/MFC etc, but don't know STL and designers that read small parts of the UI code. But indeed, if English would've used `isantropousilaus` instead of `is`, I would invent my own `iz` synonym. I'm really reluctant to reinvent my own `` utilities (which is why I asked this question in the first place), however C++'s standard library is simply not enough for modern programming, so I guess have to do that anyhow, and adding a `contains` template is really worth it. Uh, I wish the base Qt would be part of the C1x standard... — Elazar Leibovich, Mar 28 '11 at 21:16
Should't it rather be `s.find(elt) != s.end()` there (and the same goes for the `#define`)? — Fabio A., Dec 02 '11 at 22:02

score 4 · Accepted Answer · answered Mar 15 '11 at 07:11

4

If your concern is primarily terseness, then I'd suggest:

if (s.count(val)) {
  // count == 1 == true, element exists
} else {
  // count == 0 == false, element does not exist
}

But personally I still prefer checking find against end since the intent is more explicit. It's worth a bit of extra typing to me.

answered Mar 15 '11 at 07:11

Tyler McHenry

74,820
18
121
166

For readability I'd still use `0==s.count(val)`, to reduce surprise factor of the next reader (huh? count returns bool?). Plus it doesn't work for maps. But nice and creative! +1 – Elazar Leibovich Mar 15 '11 at 07:13
1

Yeah but if you're going to write out `s.count(val) == 0`, you might as well spring for the extra five keystrokes and use find/end. Also, it works fine for maps -- there is a `std::map::count` method. – Tyler McHenry Mar 15 '11 at 07:15
I know, that's why it's not the greatest way for me. And of course, `std::map::count` exists, sorry. – Elazar Leibovich Mar 15 '11 at 07:25

score 2 · Answer 2 · answered Mar 15 '11 at 07:12

2

if (s.find(val) == s.end()) is the 'standard' way of checking if an item exists in a container. As far as I'm aware, neither Boost or the standard library offer any sort of function that returns a bool denoting whether or not an element exists in a container.

If you want to do this, you would have to implement it yourself. It just doesn't make sense for the standard library or Boost to provide existing functionality that just looks different.

If you wanted to do this, the templated implementation would be the best solution. It's usually not a good idea to use defines in C++.

answered Mar 15 '11 at 07:12

Collin Dauphinee

13,664
1
40
71

@dauphic, I hate to say that, but the fact that "It just doesn't make sense for the standard library or Boost to provide existing functionality that just looks different." is really false. Why isn't vector.find a specialization of std::find? What about BOOST_FOREACH? Actually the whole point of readability is implementing existing functionality in a terse readable way. Write code for you to read, not for the computer to execute... – Elazar Leibovich Mar 15 '11 at 07:19
`std::vector` has no `find` function. The purpose of `BOOST_FOREACH` is to reduce tedious boilerplate code, not necessarily to make it easier to read, though it does that as a side-effect; if you aren't using typedefs, you can often have iterator types that are more than 120 characters in length. Comparing a returned iterator to `end` is not hard to read, it's __the standard way of checking if an element exists in a container__. Any C++ developer should know exactly what this code is doing without giving it any thought. – Collin Dauphinee Mar 15 '11 at 07:56
Variable names can also get long. Just explain me why is BOOST_FOREACH considered boilerplate-reducer, and contains(x,y) is not. Both do the same thing, type less, and have your code look more like english. Sorry, indeed vector has no find, but map do, and can be implemented with specialization on find. What about `empty()` and `size()==0`? It's simply more readable. What about `map::operator[]`, it's equivalent to `(*((this->insert(make_pair(x,T()))).first)).second`. I'm sure I can find more similar examples. – Elazar Leibovich Mar 15 '11 at 09:38
@dauphic, oh, and the standard way of iterating an object with STL is `for (auto it(o.begin());it!=o.end();++it)`, which everyone who's using the STL should be familiar with, yet, we need BOOST_FOREACH, for brevity/readability. – Elazar Leibovich Mar 15 '11 at 09:40
@Elazar: That's C++0x, where a better way would be range-for (built-in BOOST_FOREACH). – visitor Mar 15 '11 at 09:49
@visitor, the `auto` doesn't matter, I was just to lazy to write `container::iterator`, the thing is that BOOST_FOREACH is redundant but more readable than the `textbook` way. – Elazar Leibovich Mar 15 '11 at 09:52
@dauphic, `min` is simply `*min_element`, `copy_backwards` is copy with reverse iterators, do you want me to go on? – Elazar Leibovich Mar 15 '11 at 10:17

Readable alternative to std::set::find

2 Answers2