Is this code abuse STL find_if?

Question

Say I have a list of server names stored in a vector, and I would like to contact them one at a time until you answer successfully. I thought about using the STL find_if algorithm as follows:

find_if(serverNames.begin(), serverNames.end(), ContactServer()); 

Where ContactServer is a predicate function object.
On the one hand, there is a problem because the predicate does not always return the same result for the same server name (due to server downtime, network problems, etc.). However, the same result will be returned regardless of which copy of the predicate is used (i.e., the predicate has no real state), so the original problem with state predicates is not relevant in this case.

What do you say?

Answer 1

I think I'll go after him.

The only thing I will worry about is readability (and therefore maintainability). For me, he reads something like "Find the first server I can contact", which makes sense.

You might want to rename ContactServer to indicate that it is a predicate; CanContactServer ? (But then people will complain about hidden side effects. Hmm ...)

Answer 2

This is exactly what the STL algorithms are for. This is not an abuse. It is also very readable. Forward to zero someone who tells you otherwise.

Answer 3

In my opinion, this use of std::find_if bit misleading. When I read this piece of code, I do not expect any side effects, I just expect the server name to be found. The fact that the result of find_if will be discarded will also make me wonder if the code is really correct. Perhaps a different name for the predicate will make the goal clearer, but I think the problem is more fundamental.

For most people, find_if is a query , not a modifying algorithm. Despite the fact that you do not actually change the values ​​that are repeated, you change the global state of your application (in this case, you can even change the state of remote servers).

In that case, I would probably stick with a manual loop, especially now that C ++ 11 introduced range-based loops:

 for (std::string const & name : serverNames) { if (ContactServer(name)) break; } 

Another solution would be to encapsulate this in a function with a name that more clearly displays the intent, like apply_until or something like this:

 template <typename InputIterator, typename Function> void apply_until(InputIterator first, InputIterator last, Function f) { std::find_if(first, last, f); // or // while (first != last) // { // if (f(*first)) break; // // ++first; // } } } 

But maybe I'm too purist :)!

Answer 4

Isn't that what find_if for?

Note that it will find all servers if you go through the iterator, but you do not (according to OP).

Answer 5

However, the same result will be returned regardless of which copy of the predicate is used (i.e., the predicate does not have a real state), therefore, the original problem with state predicates is not relevant in this case.

So where is the problem? Function objects do not have to be state. It is best to use function objects instead of function pointers in such situations, because compilers embed them better. In your case, there is no overhead for the instance and call of the function object, since find_if is the function template, and the compiler will generate its own version for your functor.

On the other hand, using a function pointer will lead to indirectness.

Answer 6

In the upcoming version of the C ++ standard, I could not find any obvious restrictions regarding the fact that the predicate should always return the same value for the same input. I reviewed section 25 (paragraphs 7-10).

Methods that return values ​​that can change from one call to another, as in your case, should be unstable (from 7.1.6.1/11: "volatile is a hint of implementation to avoid aggressive optimization associated with the object, since the value the object can be modified using tools that cannot be detected through implementation ").

Predicates "must not use any inconsistent function through dereferenced iterators" (paragraphs 7 and 8). I believe this means that they are not required to use non-volatile methods and that your use case is thus in line with the standard.

If the wording “predicates should use the const ... functions” or something like that, I would conclude that the “const volatile” functions were out of order. But this is not so.

Answer 7

std :: for_each may be the best candidate for this.

1) After copying to the same functional object, it is used for each element, and after all the elements have been processed, a copy of a potentially updated functional object is returned to the user.

2) It would also improve call readability in my opinion.

The function object and the for_each call look something like this:

 struct AttemptServerContact { bool server_contacted; std::string active_server; // name of server contacted AttemptServerContact() : server_contacted(false) {} void operator()(Server& s) { if (!server_contacted) { //attempt to contact s //if successful, set server_contacted and active_server } } }; AttemptServerContact func; func = std::for_each(serverNames.begin(), serverNames.end(), func); //func.server_contacted and func.active_server contain server information. 

Answer 8

find_if seems to be the right choice here. In this situation, the predicate has no state.