www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Should reduce take range as first argument?

reply Justin Whear <justin economicmodeling.com> writes:
In its current form, std.algorithm.reduce takes optional seed value(s) 
for the accumulator(s) as its first argument(s). This breaks the nice 
chaining effect made possible by UFCS:

Works:
-----------------------------------------
auto foo = reduce!((string s, string x) => s ~= x)("BLAH", args.map!(x => 
x[1..$-1]));
-----------------------------------------

Doesn't work, but looks much nicer:
-----------------------------------------
auto foo = args.map!(x => x[1..$-1]))
               .reduce!((string s, string x) => s ~= x)("BLAH");
-----------------------------------------

This could be fixed with a breaking change by making the subject range to 
be the first parameter. Aside from breaking existing code, are there 
other obstacles to changing this?

Justin Whear
May 14 2012
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 5/14/12 4:10 PM, Justin Whear wrote:

 In its current form, std.algorithm.reduce takes optional seed value(s)
 for the accumulator(s) as its first argument(s). This breaks the nice
 chaining effect made possible by UFCS:

 Works:
 -----------------------------------------
 auto foo = reduce!((string s, string x) =>  s ~= x)("BLAH", args.map!(x =>
 x[1..$-1]));
 -----------------------------------------

 Doesn't work, but looks much nicer:
 -----------------------------------------
 auto foo = args.map!(x =>  x[1..$-1]))
                 .reduce!((string s, string x) =>  s ~= x)("BLAH");
 -----------------------------------------

 This could be fixed with a breaking change by making the subject range to
 be the first parameter. Aside from breaking existing code, are there
 other obstacles to changing this?

 Justin Whear


Yah, reduce was not designed for the future benefit of UFCS. (I recall 
take() was redefined towards that vision, and it was a good move.)

We can actually deprecate the current order and accept both by inserting 
the appropriate template constraints. We change the documentation and 
examples to reflect the new order, and we leave a note saying that the 
old order is deprecated. We can leave the deprecated version in place 
for a long time. Thoughts?


Andrei
May 14 2012
next sibling parent "Mehrdad" <wfunction hotmail.com> writes:
On Monday, 14 May 2012 at 21:33:19 UTC, Andrei Alexandrescu wrote:

 We can actually deprecate the current order and accept both by 
 inserting the appropriate template constraints. We change the 
 documentation and examples to reflect the new order, and we 
 leave a note saying that the old order is deprecated. We can 
 leave the deprecated version in place for a long time. Thoughts?



Just need to be careful with the constraints...
May 14 2012
prev sibling next sibling parent Timon Gehr <timon.gehr gmx.ch> writes:
On 05/14/2012 11:33 PM, Andrei Alexandrescu wrote:

 On 5/14/12 4:10 PM, Justin Whear wrote:

 In its current form, std.algorithm.reduce takes optional seed value(s)
 for the accumulator(s) as its first argument(s). This breaks the nice
 chaining effect made possible by UFCS:

 Works:
 -----------------------------------------
 auto foo = reduce!((string s, string x) => s ~= x)("BLAH", args.map!(x =>
 x[1..$-1]));
 -----------------------------------------

 Doesn't work, but looks much nicer:
 -----------------------------------------
 auto foo = args.map!(x => x[1..$-1]))
 .reduce!((string s, string x) => s ~= x)("BLAH");
 -----------------------------------------

 This could be fixed with a breaking change by making the subject range to
 be the first parameter. Aside from breaking existing code, are there
 other obstacles to changing this?

 Justin Whear


 Yah, reduce was not designed for the future benefit of UFCS. (I recall
 take() was redefined towards that vision, and it was a good move.)

 We can actually deprecate the current order and accept both by inserting
 the appropriate template constraints. We change the documentation and
 examples to reflect the new order, and we leave a note saying that the
 old order is deprecated. We can leave the deprecated version in place
 for a long time. Thoughts?


 Andrei


Just as for take, the new order is backwards. I don't know what to do 
about it though.

Also, what would be the deprecation path for reduction of a range of 
this form:

class Foo{
     Foo front();
     bool empty();
     void popFront();
}

:o)
May 14 2012
prev sibling parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Monday, 14 May 2012 at 21:33:19 UTC, Andrei Alexandrescu wrote:

 Yah, reduce was not designed for the future benefit of UFCS. (I 
 recall take() was redefined towards that vision, and it was a 
 good move.)

 We can actually deprecate the current order and accept both by 
 inserting the appropriate template constraints. We change the 
 documentation and examples to reflect the new order, and we 
 leave a note saying that the old order is deprecated. We can 
 leave the deprecated version in place for a long time. Thoughts?


 Andrei


[Resurrecting old thread]

I've into somebody else having this issue in .Learn very 
recently. I was *going* to propose this change myself, but 
happened on this thread directly. I think it is a good idea.

Looks like nobody tackled this since. Is it OK if I go ahead and 
implement this?

Just ot be clear, the goal is to support both:
reduce(range, seed) and reduce(seed, range)
Then we make reduce(seed, range) deprecated
Then we remove it.

I'd rather have a green light here, then force the discussion on 
a pull request... :D
Oct 04 2012
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Thursday, 4 October 2012 at 07:25:20 UTC, monarch_dodra wrote:

 I'd rather have a green light here, then force the discussion 
 on a pull request... :D


Well, I went and implemented the option of doing it Range first, 
then Seed, so that UFCS works.

https://github.com/D-Programming-Language/phobos/pull/861

Whether or not we deprecate the old syntax, I guess, is still up 
to debate.
Oct 11 2012
parent reply "bearophile" <bearophileHUGS lycos.com> writes:
monarch_dodra:


 Well, I went and implemented the option of doing it Range 
 first, then Seed, so that UFCS works.


To reduce deprecation troubles there is a Bugzilla suggestion to 
call it fold():
http://d.puremagic.com/issues/show_bug.cgi?id=8755

Bye,
bearophile
Oct 11 2012
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Thursday, 11 October 2012 at 11:17:51 UTC, bearophile wrote:

 monarch_dodra:


 Well, I went and implemented the option of doing it Range 
 first, then Seed, so that UFCS works.


 To reduce deprecation troubles there is a Bugzilla suggestion 
 to call it fold():
 http://d.puremagic.com/issues/show_bug.cgi?id=8755

 Bye,
 bearophile


Hum...

I can go either way. Both have their ups and downs.

*fold:
**No ambiguity during a migration
**No ambiguity regarding argument ordering
**Duplicates function names
**Changes a (relatively) established function name.
**Higher code impact if we deprecate reduce

*reduce
**Will create some ambiguity, as both reduce(r,s) and reduce(s, 
r) will be valid.
**Will only impact reduce with seed if we deprecate the old 
ordering.

I wouldn't mind getting a nudge in the right direction if anybody 
has a stance on this (Andrei?)
Oct 11 2012
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Thursday, 11 October 2012 at 12:34:17 UTC, monarch_dodra wrote:

 On Thursday, 11 October 2012 at 11:17:51 UTC, bearophile wrote:

 monarch_dodra:


 Well, I went and implemented the option of doing it Range 
 first, then Seed, so that UFCS works.


 To reduce deprecation troubles there is a Bugzilla suggestion 
 to call it fold():
 http://d.puremagic.com/issues/show_bug.cgi?id=8755

 Bye,
 bearophile


 Hum...

 I can go either way. Both have their ups and downs.

 *fold:
 **No ambiguity during a migration
 **No ambiguity regarding argument ordering
 **Duplicates function names
 **Changes a (relatively) established function name.
 **Higher code impact if we deprecate reduce

 *reduce
 **Will create some ambiguity, as both reduce(r,s) and reduce(s, 
 r) will be valid.
 **Will only impact reduce with seed if we deprecate the old 
 ordering.

 I wouldn't mind getting a nudge in the right direction if 
 anybody has a stance on this (Andrei?)


Anyone?

The pull has been open for a bit more than a month now, with no 
feedback...
Nov 29 2012
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/29/12 11:02 AM, monarch_dodra wrote:

 Anyone?

 The pull has been open for a bit more than a month now, with no feedback...


I think it's a good idea, will look at the diff.

Andrei
Nov 29 2012