[porges]

> After instructions, participants were given printouts of sample code they could refer to while solving tasks. Group Lambda got code of a C++ program using lambda expressions and group Iterator received code of the same program written using iterators. They then had time to study the samples before starting the tasks and could refer to these samples later.

These samples do not appear in the paper, so we don't know what they saw.

The “iterators” discussed are Java-/C#-style iterators, not C++ ones (as I expected reading the abstract).

In a C++ context I would have expected lambdas vs iterators to be something like:

    // lambda
    float retVal = 0;
    std::for_each(mb.cbegin(), mb.cend(), [&](item x) { retVal += item.price; });
    return retVal;

    // pure iterator
    float retVal = 0;
    for (auto it = mb.cbegin(); it != mb.cend(); ++it)
    {
        retVal += it->price;
    }

    return retVal;

... and the first would be better off as:

    return std::accumulate(mb.cbegin(), mb.cend(), 0f,
        [](float acc, item x) { return acc + x.price; });

I think the need to use ref-capture (since you only get a side-effecting `std::function` to play with in their sample) would be the thing most likely to throw people off – as it’s something that should be avoided in most code, anyway ;)

[MaulingMonkey]

Yeah - from what I can see, neither interface looks like idiomatic C++.

EDIT: Looks like you beat me to the punch on some of these ;)

Instead of:

  float getSum(marketBasket mb) {
    float retVal = 0;
    // Implement solution here
    // ---------
    marketBasket::iterator iter = mb.begin();
    while (iter.hasNext()) {
      retVal += iter.get().price;
      iter.next();
    }
    // ---------
    return retVal;
  }

I'd rather see real SC++L compatible iterators (as hopefully taught) and saner naming:

  float getSum(marketBasket market) {
    float sum = 0;
    // Implement solution here
    // ---------
    for (marketBasket::iterator item = market.begin(); item != market.end(); ++item) {
      sum += item->price;
    }
    // ---------
    return sum;
  }

And instead of being pre-provided with a function <void(item)>, if I'm reading the pdf correctly:

  float getSum(marketBasket mb) {
    float retVal = 0;
    // Implement solution here
    // ---------
    function <void(item)> func = [&](item theItem) {
      retVal += theItem.price;
    };
    mb.iterateOverItems(func);
    // ---------
    return retVal;
  }

I'd rather see:

  float getSum(marketBasket market) {
    float sum = 0;
    // Implement solution here
    // ---------
    market.for_each([&](item theItem) {
      sum += theItem.price;
    });
    // ---------
    return sum;
  }

Or venturing into the far more functional style, where lambdas start to shine for me, personally:

  float getSum(std::vector<item> market) {
    // Implement solution here
    // ---------
    return std::accumulate(market.begin(), market.end(), 0.0f, [&](float sum, item theItem) {
      return sum + theItem.price;
    });
    // ---------
  }

It looks a bit better in C# where your selection of standard functions is a little less anemic and a little nicer to use:

  float GetSum(MarketBasket market) {
    // Implement solution here
    // ---------
    return market.Sum(item => item.Price);
    // ---------
  }

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

[Karliss]

First two tests used lambda with custom for each method. If authors used real c++ iterators and for each loop it would become obvious that comparing iterators and lambdas at iterating is the same as comparing lambdas with if statements at being if statements.

[72deluxe]

I agree that the "iterators" in the paper are not C++ iterators as found in the STL or language spec.