> "Idiomatic" doesn't necessarily mean better. I think objectively it's hard to argue that "item != market.end()" is superior to "iter.hasNext()". The latter accurately reflects the programmer's intent, while the former specifies an unnecessarily specific (and poor) implementation of the intent
At this point, I find the former much more readable than the latter.
I have seen the former a thousand times. I have seen the latter once - here, and here alone. Oh sure, I've seen similar patterns - under different names, usually under different languages - but I'm pretty sure this is exactly the first time I've ever seen it named "hasNext".
The worst bit is mixing SC++L terms - "::iterator" - and foreign idioms that do NOT conform to the SC++L concept of what I expect an "::iterator" to do. It is surprise and confusion of the worst kind. Even relatively inexperienced C++ programmers that I know would pause as I did, and ask - "Wait, what the fuck?", and then waste the next few minutes rediscovering what the hell the code is doing (absolutely nothing special that would call for special divergence from the norm.)
As such I must thoroughly disagree on the strength of the communication of intent between the two samples. The former might as well be plain English to me - the latter might as well be pig latin. Oh sure, I can figure it out - but it'll take me a minute. It DID take me a minute.
You want to steal patterns from Java or C#? Then at least name them after the Java or C# concept. Call it enumerator. Embrace the fact that you're using Java or C#'s idioms. Decry C++'s idioms as poor choices by the C++ language if you must. And hell, yes, there are exceptional cases where you can say "let's go with none of the above - the best solution is unconventional and unusual, using the normal idioms of no language."
You have very much failed to convince me that this is one of them.
And while you could perhaps argue that C#'s idioms are superior, I would note that you're a few decades late to the party on that count, and talking to the wrong guy. I wouldn't even disagree, necessarily. But C++'s idioms have a useful inertia at this point, and are frankly not that bad. Rather reasonable, even. Although they do have a learning curve.
> First of all, using something like "market.cend() != const_iter" instead is arguably better practice (imagine you unintentionally omit the "!").
I prefer to have a warning-as-error for the unparenthesized assignment you posit (if only because my coworkers probably didn't use yoda conditionals on the existing codebase). But hell, even yoda conditionals would be better.
> But programmers shouldn't need to consider whether the iterator is const or not when they just want to know if the loop is done.
Realistically, I won't - instead preferring auto, or never naming my iterators at all, as I've done in the std::accumulate example - as begin is overloaded appropriately.
> Also, consider the case where the vector is being modified (items inserted or deleted) inside the loop. It might be problematic either way, but "item != market.end()" is particularly bad in that situation.
Oh, I know what I want in this case! Iterator debugging to catch the problem. The PDF has the source code to ::hasNext:
bool marketBasket::iterator::hasNext() { return owner−>items.size() − index > 0; }
Do you see iterator debugging? I don't. What happens when index is greater than .size() because of a removed element?
Based on other code, we know items is a std::vector. And thus that .size() is unsigned. And thus that the result of ...size() - index is unsigned. And thus that the result will underflow to a huge number, and that ::hasNext() will return true, leading to a buffer overflow.
If "item != market.end()" is particularly bad, then "iter.hasNext()" must be particularly downright evil, for it's doing even worse.
Note that any fix applied to ::hasNext could equally be applied to comparing against .end().