Strange behaviour of std::find, returns true when the element is not in the vector

2386 views c++
-3

Looks a very straight forward case, a typical use of std::find

for ( auto element : generic->vec() )
        LOG << element;

    LOG << channel;

    if ( !gen->vec().empty() ) {

        if(std::find(generic->vec().begin(), generic->vec().end(), channel) != generic->vec().end()){

            LOG << "Found";
            ;// Found the item
        } else {

            LOG << "Not Found";
            return false;

        }
}

Please check the log file

2018-11-08, 09:37:18 [INFO] - [140455150589696] - 1
2018-11-08, 09:37:18 [INFO] - [140455150589696] - 2
2018-11-08, 09:37:18 [INFO] - [140455150589696] - 4
2018-11-08, 09:37:18 [INFO] - [140455150589696] - 12
2018-11-08, 09:37:18 [INFO] - [140455150589696] - 40
2018-11-08, 09:37:18 [INFO] - [140455150589696] - Found

The vector contains 1,2,4,12 and the incoming value that we want to test if it belongs to the vector is 40. The std::find returns true, that it is found.

The vec() method returns an array of uint64_t elements:

std::vector<uint64_t>  vec() const {
  return vec_;
}

When I am creating a local vector, ie

auto tmp = generic->vec(),

the code works.

Where is the bug in my code? I would expect to get "Not found" when checking if 40 belongs to [1,2,4,12].

answered question

the auto tmp = generic->vec(); is a good solution

2 Answers

9

The problem is that your vec function returns the vector by value. That means each call to vec will return a different and distinct vector object. And iterators from different vectors can't be compared to each other.

The simple solution is to return the vector by reference:

std::vector<uint64_t> const&  vec() const { ... }

posted this
0

The signature of std::vector<uint64_t> vec() const implies that an rvalue is returned (a copy of vec_). This is fine as long as you don't compare references to elements of vec() from distinct invocations. Example:

auto v1 = vec();
auto v2 = vec();

assert(v1.begin() != v2.begin()); // different objects

This is the same as

assert(vec().begin() != vec().begin());

Such things can be hard to spot when you're used to range based for loops, e.g.

for (const auto& value : vec()) { /* ... */ }

That's not a problem, but under the hood, this snippet is expanded such that the return value of vec() is bound to a auto&& variable and any calls to *begin/*end methods refer to the same object.

Long story short: bind the return value to a variable before requesting iterators or change the signature ov vec() such that it returns a reference, not a copy.

posted this

Have an answer?

JD

Please login first before posting an answer.