http://d.puremagic.com/issues/show_bug.cgi?id=8155

           Summary: Deprecate std.range.lockstep
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: Phobos
        AssignedTo: nobody@puremagic.com
        ReportedBy: bearophile_hugs@eml.cc


--- Comment #0 from bearophile_hugs@eml.cc 2012-05-27 14:20:51 PDT ---
I suggest to deprecate std.range.lockstep because with the recent improvements in tuple unpacking, std.range.zip is able to replace its the main purpose, this now works:

import std.stdio, std.range;
void main() {
    foreach (a, b; zip([1,2], ["red", "blue"]))
        writeln(a, " ", b);
}


lockstep() is also able to iterate with an index variable, zip() is not able to
do it:

import std.stdio, std.range;
void main() {
    foreach (i, a, b; lockstep([1,2], ["red", "blue"]))
        writeln(i, " ", a, " ", b);
}


But from my experience the need of an indexing variable on a zipped iteraton is not so common, and the there are other solutions, like:

import std.stdio, std.range;
void main() {
    foreach (i, a, b; lockstep(iota([1,2].length), [1,2], ["red", "blue"]))
        writeln(i, " ", a, " ", b);
}


A better solution (hopefully not too much slower) is to use enumerate(zip()),
as in Python and Haskell, see Issue 5550 :


import std.stdio, std.algorithm, std.range, std.typecons, std.traits, std.array;

struct Enumerate(R) {
    R r;
    int i;

    @property bool empty() {
        return this.r.empty;
    }

    @property Tuple!(typeof(this.i), typeof(R.init.front)) front() {
        return typeof(return)(i, this.r.front);
    }

    void popFront() {
        this.r.popFront();
        this.i++;
    }
}

Enumerate!R enumerate(R)(R range, int start=0) if (isInputRange!R) {
    return Enumerate!R(range, start);
}

void main() {
    foreach (i, a, b; enumerate(zip([1,2], ["red", "blue"])))
        writeln(i, " ", a, " ", b);
}



Or even use countFrom (similar to Python itertools.count()), see Issue 7839 :

import std.stdio, std.range;

struct CountFrom(T) {
    T n;
    this(T n_) { this.n = n_; }
    const bool empty = false;
    @property T front() { return n; }
    void popFront() { /* n++; */ n += 1; }
}

CountFrom!T countFrom(T)(T start) { return CountFrom!T(start); }
CountFrom!T countFrom(T)() { return CountFrom!T(cast(T)0); }

void main() {
    foreach (i, a, b; zip(countFrom!size_t(), [1,2], ["red", "blue"]))
        writeln(i, " ", a, " ", b);
}


(For Phobos there are much more commonly useful functions, like amap/afilter
that mean array(map()) and array(filter()) that are useful everywhere and
shorten/simplify the code).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=8155


Andrej Mitrovic <andrej.mitrovich@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrej.mitrovich@gmail.com


--- Comment #1 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2012-06-05 10:57:56 PDT ---
Sorry, but I *do* use the index variable:

https://github.com/AndrejMitrovic/DWinProgramming/blob/master/Samples/Chap06/KeyView1/KeyView1.d#L198 https://github.com/AndrejMitrovic/DWinProgramming/blob/master/Samples/Chap09/BtnLook/BtnLook.d#L144

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=8155



--- Comment #2 from bearophile_hugs@eml.cc 2012-06-05 13:45:11 PDT ---
(In reply to comment #1)
> Sorry, but I *do* use the index variable:

I didn't said that index variable is never useful and never used. I have said its need is rare, in my experience of using Python and in my experience of looking at Haskell code.


On the other hand I have shown and offered 3 different ways to solve the problem without lockstep. lockstep is redundant, and its usage doesn't shorten a significant amount of code: there are redundant operations that are way more commonly useful than lockstep, like amap/afilter.


> https://github.com/AndrejMitrovic/DWinProgramming/blob/master/Samples/Chap06/KeyView1/KeyView1.d#L198 https://github.com/AndrejMitrovic/DWinProgramming/blob/master/Samples/Chap09/BtnLook/BtnLook.d#L144

Of the 3 alternative solution, using enumerate, those:

foreach (index, button, ref hwndButton; lockstep(buttons, hwndButtons))

foreach (index, myMsg; lockstep(iota(0, min(cLines, cyClient / cyChar - 1)),
retro(msgArr)))

Become:

foreach (index, button, ref hwndButton; enumerate(zip(buttons, hwndButtons)))

foreach (index, myMsg; enumerate(zip(iota(min(cLines, cyClient / cyChar - 1)),
retro(msgArr))))

Or maybe:

foreach (index, button, ref hwndButton; buttons.zip(hwndButtons).enumerate())

foreach (index, myMsg; min(cLines, cyClient / cyChar -
1).iota().zip(msgArr.retro()).enumerate())

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=8155



--- Comment #3 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2012-06-05 14:15:51 PDT ---
Yes let's break code and make new code more verbose, great idea.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=8155



--- Comment #4 from bearophile_hugs@eml.cc 2012-06-05 15:01:26 PDT ---
(In reply to comment #3)
> Yes let's break code

You can't assume Phobos will not change. Some parts of Phobos were designed very quickly, and not at their best. We need deprecation patterns, new better things to be introduced and older less-well designed things deprecated and removed.

In hindsight the decision to add lockstep() was a mistake, it was based on a
limit of D language that Hara has quickly removed, allowing zip() to cover most
usages of lockstep(). Now lockstep() is redundant because you are able to avoid
it just calling another function.

As you know Walter prefers to not add shallow functions to Phobos. If you
assume lockstep() is not present in Phobos and you want to add it, people will
say you that zip().enumerate() (or other similarly simple solutions) is able to
replace it, so it's too much shallow to add it.

In my opinion to add a shallow function to Phobos it must replace a very often
used pattern, as  filter().array().  While I think zip().enumerate() is not one
of such very common patterns.


> and make new code more verbose,

It's a little more verbose, but the need of a index is not common when you zip iterables, so the _total_ increase of code is very small.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=8155



--- Comment #5 from bearophile_hugs@eml.cc 2012-06-05 15:04:06 PDT ---
(In reply to comment #3)
> Yes let's break code and make new code more verbose, great idea.

Your answers have made those arguments of mine stronger, so thank you for your useful comments.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=8155



--- Comment #6 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2012-06-05 15:18:38 PDT ---
(In reply to comment #4)
> (In reply to comment #3)
> > Yes let's break code
> 
> You can't assume Phobos will not change.

You might introduce a new template tomorrow, and then remove it a month later, so why should I even bother coding against such a library? Is Phobos someone's playground or a standard library?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=8155



--- Comment #7 from bearophile_hugs@eml.cc 2012-06-05 16:36:14 PDT ---
(In reply to comment #6)

> You might introduce a new template tomorrow, and then remove it a month later, so why should I even bother coding against such a library? Is Phobos someone's playground or a standard library?

Take a look at the Phobos2 changelog for DMD 2.060. There are functions and
four whole modules removed from Phobos2:
https://github.com/D-Programming-Language/phobos/blob/08a62cdaaa37fd7168bb6bf0d63f00ead1eeb4d0/changelog.dd

I think two more modules will be removed in August this year. Phobos2 is young still, it contains many mistakes, so it's better to fix it. In some years the rate of deprecation will probably decrease.

In D there is the deprecated keyword, and other things that help the deprecation process. They are meant to be used. I am not asking to remove lockstep() today, I suggest to follow a normal sane path of announcing a deprecation, deprecate it some months later (but keep it for a year or so) and then remove it.

Removing redundant/bad/broken functions/things from Phobos is useful, because it reduces the efforts to learn to use D+Phobos. A bit of growing pain is acceptable, especially in the first years, if they spare a bigger amount of pain/work/confusion later.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=8155


Joseph Rushton Wakeling <joseph.wakeling@webdrake.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |joseph.wakeling@webdrake.ne
                   |                            |t


--- Comment #8 from Joseph Rushton Wakeling <joseph.wakeling@webdrake.net> 2013-06-28 06:19:31 PDT ---
(In reply to comment #0)
> I suggest to deprecate std.range.lockstep because with the recent improvements in tuple unpacking, std.range.zip is able to replace its the main purpose

There are currently some things that work with lockstep that don't with zip. Consider:

    auto arr1 = new double[10];
    foreach(i, ref x; zip(iota(10), arr1))
    {
        x = i;
    }
    writeln(arr1);

    auto arr2 = new double[10];
    foreach(i, ref x; lockstep(iota(10), arr2))
    {
        x = i;
    }
    writeln(arr2);

The first array will output all nan's, the second will have values set correctly.  I imagine this is a bug, but it needs to be fixed before zip is a viable lockstep replacement.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=8155



--- Comment #9 from bearophile_hugs@eml.cc 2013-06-28 09:46:37 PDT ---
(In reply to comment #8)

>     auto arr1 = new double[10];
>     foreach(i, ref x; zip(iota(10), arr1))
>     {
>         x = i;
>     }
>     writeln(arr1);
> 
>     auto arr2 = new double[10];
>     foreach(i, ref x; lockstep(iota(10), arr2))
>     {
>         x = i;
>     }
>     writeln(arr2);

Let me add a note. For your specific use case it's better to use enumerate(),
from Issue 5550 :

    auto arr1 = new double[10];
    foreach (i, ref x; arr1.enumerate)
        x = i;
    arr1.writeln;

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------