std.range: explain difference between lockstep and zip

[wilzbach]

this is a follow-up to the discussions from the forum

1. I tried to add an explanation for the differences
2. I removed the foreach example for zip - this is too confusing!
3. I had to add the remaining zip unittest in the documentation as otherwise the order with See_Also would be messed up :S

[ntrel]

I think we could move this sentence to a comment at the top of the unit test, then the example doesn't need to be in the docs.

[dnadlinger]

Yep, please try to stick to documented unit tests as much as possible. From past experience (i.e. before the feature existed), the danger of bit rot is just too high otherwise.

[wilzbach]

agreed - just didn't know a nice way to fit it into the text. I will try again ;-)

[dnadlinger]

While you are at it, could you also have a look at the corresponding comment in zip? The current wording

   $(LREF zip) is similar to $(D lockstep), but it doesn't bundle its values
   and uses the $(D opApply) protocol,`

is rather confusing to me. It's lockstep that uses opApply, not zip.

Edit: Huh, GitHub didn't show those lines as being added with this commit.

[ntrel]

I don't think we should undocument this, I actually think it should come before the sort example.

[dnadlinger]

I'd keep at least one example that shows explicitly how the different subranges can be indexed. The other example just has [0], which might not be enough to tip off users as to how that works.

[ntrel]

Similar to the sort example, I suggest we rename e to tup as it's a tuple.

[dnadlinger]

Since the phrasing seems to be off for both the functions – did you somehow mix up the descriptions for the two See Also blocks?

[wilzbach]

Please have a look at example 2. There are three ways that we could recommend - they all work.
Which should we recommend? I think all three are valid and could be useful.

As we now have an entire PR about it. Let me clarify - we have locksteep only because it is faster for loops?

    import std.algorithm : equal, map;
    import std.conv: to;

    int[] a = [ 1, 2, 3 ];
    string[] b = [ "a", "b", "c" ];

    size_t idx = 0;
    foreach (tup; zip(a, b))
    {
        assert(tup[0] == a[idx]);
        assert(tup[1] == b[idx]);
        ++idx;
    }

    idx = 0;
    foreach (t1, t2; zip(a, b))
    {
        assert(t1 == a[idx]);
        assert(t2 == b[idx]);
        ++idx;
    }

    auto result = zip(a, b).map!(tup => tup[0].to!string ~ tup[1]);
    assert(result.equal(["1a", "2b", "3c"]));

Moreover I added another nice example for zip - it's taken from my D Functional Garden and I think it's a nice way to show how zip works:

// pairwise sum
auto arr = [0, 1, 2];
assert(arr.zip(arr.save.dropOne).map!"a[0] + a[1]".equal([1, 3]));

[ntrel]

LGTM 👍

[wilzbach]

@ntrel - thanks for your helpful feedback and review.

The examples now look a lot better:

http://dtest.thecybershadow.net/artifact/website-23160046fb7ccd78e54d467d28bdb87a4c8afd25-53cf8893d4f37218a84d349547178895/web/phobos-prerelease/std_range.html#.zip

side note: the formatting of the ddocs output could be improved a bit (e.g. no duplicates "Examples")- see issue 15769

[JakobOvrum]

The actual reason lockstep isn't deprecated is because it allows reference access to elements:

auto left = new int[](3);
auto right = new int[](3);
foreach(ref lhs, ref rhs; lockstep(left, right))
{
    lhs = 1;
    rhs = 2;
}
assert(left == [1, 1, 1]);
assert(right == [2, 2, 2]);

External iteration ala zip is superior in every other respect.

[wilzbach]

The actual reason lockstep isn't deprecated is because it allows reference access to elements:

Okay then I will update the docs to point this out even more clearly :)

Btw having idexing in foreachs opCalls doesn't work with zip yet, but this might be easy to add?

[JakobOvrum]

What do you mean? If you mean iterating with an index, std.range.enumerate is designed to work well with zip.

[wilzbach]

What do you mean? If you mean iterating with an index, std.range.enumerate is designed to work well with zip.

Yes I know, but I was more referring that lockstep support the "direct" style via overloads::

    import std.stdio;
    import std.range;
    auto a = [1, 2, 3];
    auto b = ["a", "b", "c"];
    foreach(i, t1, t2; zip(a, b).enumerate)
    {
        writeln(i, t1, t2);
    }

    foreach(i, t1, t2; lockstep(a, b))
    {
        writeln(i, t1, t2);
    }

[wilzbach]

Okay then I will update the docs to point this out even more clearly :)

Updates:

• added a similar example example with ref to locksteps documentation
• added info about access with references to the explanation about differences (thanks @JakobOvrum)
• replaced values with elements

[ntrel]

Here I suggest we use e1, e2 as they're not tuples.

[wilzbach]

done - I also renamed the loop parameter for lockstep

[ntrel]

@greenify :

I was more referring that lockstep support[s] the "direct" style via overloads:
foreach(i, t1, t2; lockstep(a, b))

I think that's due to lockStep's opApply. I'm not sure that tuple unpacking for zip would work correctly if there was an index element but no index variable.

BTW foreach tuple unpacking isn't documented AFAICT:
https://issues.dlang.org/show_bug.cgi?id=7361

[wilzbach]

So I think it's better to not document this anti-feature.

That's the wrong way in my opinion, if it is slow due to the tuple expansion, we should either

• document that
• disable it

and not hope that people might not try it.

[ntrel]

added a similar example example with ref to locksteps documentation

There already was an example with ref ;-) Please see my pull which tweaks some things and also avoids docstring examples: wilzbach#1

[wilzbach]

Please see my pull which tweaks some things and also avoids docstring examples: greenify#1

Thanks @ntrel - merged with this branch :)

[ntrel]

Simplify zip examples more: greenify#1 greenify#3

[wilzbach]

Should I squash the commits?

[ntrel]

Sure, go ahead.

[wilzbach]

squashed and rebased to master.

@ntrel - it's a shame git doesn't allow joint commit messages :/

[wilzbach]

ping @ reviewers - @ntrel and I have worked on this minor update to the documentation which yields a better understanding of the difference between lockstep and zip to the reader and adds better examples. Imho this trivial PR is ready now - any other thoughts or blocking issues?

[ntrel]

@greenify I split out the obvious example changes into a separate PR but kept your authorship for the commit, hope this is OK: #4078. I left out anything which might need more reviewing.

[wilzbach]

hope this is OK

Absolutely - you could/can change it back to your name. I don't mind either.

Thanks for pushing this @ntrel, but I guess the main reason the inactivity is due to the weekend?

[JakobOvrum]

Auto-merge toggled on