6

Take the following example taken from the cplusplus.com reference page and altered to return false:

// find_if example
#include <iostream>     // std::cout
#include <algorithm>    // std::find_if
#include <vector>       // std::vector

bool IsOdd (int i) {
  return ((i%2)==1);
}

int main ()
{
  std::vector<int> myvector;    
  myvector.push_back(10);
  myvector.push_back(20);
  myvector.push_back(40);
  myvector.push_back(50);

  std::vector<int>::iterator it = std::find_if (myvector.begin(), myvector.end(), IsOdd);
  std::cout << "The first odd value is " << *it << '\n';

  return 0;
} 

Since no value in myvector is odd it will return InputIterator last, which is undefined:

The first odd value is -1727673935

What is the proper way to handle this output?

How can I know std::find_if() returned false if the output is unpredictable and comparing to the entire vector to confirm the resulting value doesn't exist defeats the purpose of using std::find_if() to begin with?

A__
  • 1,616
  • 2
  • 19
  • 33

4 Answers4

10

Do you mean

std::vector<int>::iterator it = std::find_if (myvector.begin(), myvector.end(), IsOdd);

if ( it != myvector.end() )
{
    std::cout << "The first odd value is " << *it << '\n';
}
else
{
    // std::cout << "there is no odd value in the vector\n";
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
6

The idiomatic way to do this is to check whether the iterator equals the end sentinel.

auto it = std::find_if (myvector.begin(), myvector.end(), IsOdd);
if (it == myvector.end()) {
    std::cout << "No odd values found" << std::endl;
} else {
    std::cout << "The first odd value is " << *it << std::endl;
}

In C++17 (the most recent standard), you can declare the iterator right in the if statement:

if (auto it = std::find_if(myvector.begin(), myvector.end(), IsOdd); it != myvector.end()) {
    std::cout << "The first odd value is " << *it << std::endl;
} else {
    std::cout << "No odd values found" << std::endl;
}
Brennan Vincent
  • 10,736
  • 9
  • 32
  • 54
5

std::find_if returns(reference cppreference.com)

Iterator to the first element satisfying the condition or last if no such element is found.

That means, dereference the iterator only when its not equal to container.end() iterator.

if (const auto iter = std::find_if(myvector.cbegin(), myvector.cend(), IsOdd); // need C++17 compiler support
    iter != myvector.cend())
{
    std::cout << *iter << "\n";
}
else
{
    // code
}

PS: In modern C++, lambdas expressions should be your good friends, and use it when it is appropriate. See more here: Why can lambdas be better optimized by the compiler than plain functions?

That means your IsOdd could have been

constexpr auto isOdd = [](const int i) /* noexcept */ { return i & 1; };
JeJo
  • 30,635
  • 6
  • 49
  • 88
  • 1
    The verbosity of a lambda is not useful or needed here – Lightness Races in Orbit May 27 '19 at 12:19
  • @LightnessRacesinOrbit Admit the fact. Usually, there will be a nitpicking *unrelated* comment on this kind of questions, that "hey why don't you use lambdas instead of a free-function?"; There is no one like that. Hence thought to provide the OP an extra tip/ info along with the answer, in-case he/ she is not aware of that. Sorry if it is too verbose and misleading. – JeJo May 27 '19 at 12:58
  • Just an observation on your remark "use it whenever it's possible", which seems like an overly strong recommendation. "Use it when it's appropriate" would be better, if a little self-evident. – Lightness Races in Orbit May 27 '19 at 13:00
5

You need to check if the returned iterator is the end iterator you passed to std::find_if (the second argument). These semantics are quite common for algorithms in the standard library, so you should get used to this.

const auto firstOdd = std::find_if (myvector.cbegin(), myvector.cend(), IsOdd);

if (firstOdd != myvector.cend())
    std::cout << "The first odd value is " << *it << '\n';
else
    std::cout << "No odd values found\n";

Note also that you can use the cbegin()/cend() member functions, as you're not mutating the container.

lubgr
  • 37,368
  • 3
  • 66
  • 117
  • 2
    "*compares equal to **the one-past-the-end iterator** of the container you're searching through*" - it is more accurate to say that it should be "the end iterator that you pass to find_if()" instead, which in this particular case *happens* to be the one-past-the-end iterator of the container, but that is not always the case when working with iterators in general. – Remy Lebeau May 06 '19 at 16:54
  • @RemyLebeau Thanks for the hint, that makes sense indeed. I improved the wording. – lubgr May 06 '19 at 17:00