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

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

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

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)

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

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.

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

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?)

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

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