CODEWITHSUNDEEP
c++std
Julen
Julen

Reputation: 1604

C++, find_if not working

I am using for the first time std::find_if function in C++ code. The examples and the logic I want to execute are pretty simple but somehow I cannot make it work.

I have created the "finder" class this way:

  /**
  * @class message_by_id_finder
  * @brief A class to find the matching lane wrapper
  */
  class message_by_id_finder
  {
  public:
  /** @brief constructor */
  explicit message_by_id_finder(int id) :
  m_myMessage(id) {
  }

  /** @brief the comparing function */
  bool operator()(const AppMessage& message) const {
  return message.messageId == m_myMessage;
  }

  private:
  /// @brief The id of the searched object
  int m_myMessage;
  };

Then I use it the following way: 

 // Loop messages 
for (vector<AppMessage>::iterator it = messages.begin(); it != messages.end() ; ++it ) {
// Match message with the registered by the App
AppMessage m = *it;
vector<AppMessage>::iterator it2 = find_if(m_messages.begin(), m_messages.end(), message_by_id_finder(m));
if (it2 != m_messages.end()) {
  // FOUND!
} else {
  // NOT FOUND
}
}

I looped the m_messages vector and there are members that match the id but it2 is always 0x00. Am I doing something particulary wrong?

Thank you very much in advance.

PD: Just in case, other piece of codes useful to understand the problem:

  /**
  * @struct AppMessage
  * @brief Information of a message carrying a result of the App.
  */
  struct AppMessage {
      int messageId;      
      float payloadValue;    
  };

Upvotes: 0

Views: 2066

Answers (3)

Kleist
Kleist

Reputation: 7985

Your predicate should take an object of the type iterated over, that is an AppMessage in your case.

So if you replace your operator() with something like this, I think you'll get closer to something working:

bool operator()(const AppMessage& am) const {
    return am.messageId == m_myMessage;
}

Upvotes: 2

icecrime
icecrime

Reputation: 76755

You have to understand what find_if does internally in order to use it correctly. The cplusplus reference site offers some basic code snippets which may help understand what an algorithm actually does (but remember it's just 'pseudo-code' for educational purpose, not the actual implementation). This is what this site gives as a description of std::find_if :

template<class InputIterator, class Predicate>
InputIterator find_if ( InputIterator first, InputIterator last, Predicate pred )
{
  for ( ; first!=last ; first++ ) if ( pred(*first) ) break;
  return first;
}

What you can see is that the predicate is called for each element in the sequence. Here, you have a std::vector<AppMessage> so the provided predicate should be callable with an AppMessage.

Changing you predicate to this should do the trick :

class message_by_id_finder
{
public:
    /** @brief constructor */
    explicit message_by_id_finder(int id) :
        m_myMessage(id)
    {}

    /** @brief the comparing function */
    bool operator()(const AppMessage &appMessage) const {
        return appMessage.messageId == m_myMessage;
    }

private:
    /// @brief The id of the searched object
    const int m_myMessage;
}

Also note that I made operator() and m_myMessage const (why ? because I can !).

Upvotes: 2

CashCow
CashCow

Reputation: 31435

Your boolean logic is wrong for find_if which is most likely leading to your error.

As it is, if your vectors are not very short and you are doing this a lot of times, find and find_if are very inefficient as they are linear complexity. Yours uses 2 loops which makes it O(M*N) where M and N are the length of your collections.

Big-O, as it is called, is one of the main keys to efficient programming.

If your collections are both sorted, set_intersect is the way to get all the items that are in both collections which is O(M+N). If they are not, you can sort one of them then subsequently your lookup will be O(M log N) or O(N log M) dependent on which one you sorted. If one is much longer than the other and you sort the longer one, O(M log N) is more efficient than O(M + N) (typical situation is searching for a few items in a database table that has a lot of records. Even running the table once is going to be inefficient compared to a few indexed lookups).

Upvotes: 0

Related Questions