[duneroadrunner]

"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. First of all, using something like "market.cend() != const_iter" instead is arguably better practice (imagine you unintentionally omit the "!"). 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. 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.

Shameless plug: http://duneroadrunner.github.io/SaferCPlusPlus/#msevector

[asveikau]

> I think objectively it's hard to argue that "item != market.end()" is superior to "iter.hasNext()"

Until you realize the purpose of the weird syntax in the former.

Namely: you can write algorithms where an iterator is a drop-in replacement for a simple loop through all items in an array via pointer arithmetic, if you write it to take a start and an end iterator as templates.

Plus said algorithm can take sub-ranges instead of requiring it to loop through all items.

I thought c++ iterators were very strange for many years until I understood this and other rationales.

[duneroadrunner]

Ah yes, that makes total sense. I suppose those that would like a nicer interface can just implement one on top. Like the proposed array_view<> and span<> interfaces.

[lorenzhs]

> 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.

and they don't: http://en.cppreference.com/w/cpp/container/vector/end - there's an overload returning a const_iterator. You don't need to use 'cend'.

And since insertion and deletion potentially invalidate iterators, 'hasNext()' is just as bad.

[duneroadrunner]

Yeah, maybe I didn't think the const thing through. I guess I was thinking that the benefit of using cend() is the double-check to make sure the iterator was declared as a const_iterator (when appropriate). But using "hasNext()", just like using "end()", you would lose the double-check.

Perhaps I could instead claim that "item != market.end()" redundantly specifies the vector, "market", and thus presents an unnecessary opportunity for mistakes? For example:

    std::vector<double> x_coords;
    std::vector<double> y_coords;
    ...
      
    for (auto y_iter = y_coords.begin(); y_iter != y_coords.end(); y_iter++) {
        for (auto x_iter = x_coords.begin(); x_iter != y_coords.end(); x_iter++) {
            ...
        }
    }

Notice the "x_iter != y_coords.end()" bug. I assume this will usually trip an assert if it is encountered in debug mode, but not in release mode. Of course you could just as easily mix up "x_iter.hasNext()" with "y_iter.hasNext()", but the removal of redundancy means one less opportunity for a potential mistake. Right? Hmm, I guess that's really an argument for losing the iterators altogether, which I guess was kind of the point of the study.

So then wrt iterator invalidation, I have a question. Consider this contrived scenario:

    for (auto y_iter = y_coords.begin(); y_iter != y_coords.end(); y_iter++) {
        if (5 == std::distance(y_coords.begin(), y_iter)) {
            y_coords.resize(3);
        }
    }

With conventional implementations of std::vector, the "y_coords.resize(3)" will presumably "invalidate" y_iter. And the "y_iter != y_coords.end()" will result in undefined behavior. But you could imagine "safer" implementations of vector<> that would instead throw an exception (or terminate or whatever). (Or you could actually download one of them at the link I gave.) So the question is, if this "safer" implementation supported "y_iter.hasNext()", would it be better for it to throw an exception (or whatever) in this case, or just return false?

[MaulingMonkey]

> So the question is, if this "safer" implementation supported "y_iter.hasNext()", would it be better for it to throw an exception (or whatever) in this case, or just return false?

The safest option is to compile this code as Rust and have it fail to satisfy the borrow checker. And basic syntax parsing because this is C++ - but ehh, details.

Certain static analysis tools may also catch the issue. Well, if you're using real C++ iterators and not ::hasNext at least.

[duneroadrunner]

Sorry never mind, in that last scenario it's the "y_iter++" that will be the first invalid operation.

[MaulingMonkey]

> "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().

[duneroadrunner]

> You have very much failed to convince me that this is one of them.

Yeah, after further consideration I'm not convinced either.