December 16, 2014
>>
>> so it will be documented? that was the rhetorical question.
>
> Does it need to be? I don't see a reason for anyone to go out of their way to make the implementation inconsistent. Do you?

At least I would prefer not to rely on undefined behaviour. If we ever do change the AA implementation in an inconsistent way, it will at least prevent the "but this code was broken all along"-argument.

On the other hand adding just one sentence to the documentation would cost us nothing.
December 16, 2014
On 12/16/14 10:33 AM, Tobias Pankrath wrote:
>
>>>
>>> so it will be documented? that was the rhetorical question.
>>
>> Does it need to be? I don't see a reason for anyone to go out of their
>> way to make the implementation inconsistent. Do you?
>
> At least I would prefer not to rely on undefined behaviour. If we ever
> do change the AA implementation in an inconsistent way, it will at least
> prevent the "but this code was broken all along"-argument.
>
> On the other hand adding just one sentence to the documentation would
> cost us nothing.

I can never ever see a reason to implement 2 different ways to traverse the elements, just to piss off people?

If you make a PR that adds that to documentation, I will pull it if it makes you feel better. I don't think it hurts, but don't think it's worth my time to make such a PR.

-Steve
December 16, 2014
On Monday, 15 December 2014 at 23:21:44 UTC, bearophile wrote:
> Nordlöw:
>
>> BTW: Why doesn't aa.byKey.map work?
>
> It currently works.
>
> Bye,
> bearophile

My mistake. Now both are at

https://github.com/nordlow/justd/blob/master/range_ex.d#L527
December 16, 2014
On Tuesday, 16 December 2014 at 16:08:09 UTC, Steven Schveighoffer wrote:
> I can never ever see a reason to implement 2 different ways to traverse the elements, just to piss off people?
>
> If you make a PR that adds that to documentation, I will pull it if it makes you feel better. I don't think it hurts, but don't think it's worth my time to make such a PR.
>
> -Steve

I can do PR for adding

https://github.com/nordlow/justd/blob/master/range_ex.d#L527

to Phobos.

Were should I put it/them?
December 16, 2014
On 12/16/14 11:47 AM, "Nordlöw" wrote:
> On Tuesday, 16 December 2014 at 16:08:09 UTC, Steven Schveighoffer wrote:
>> I can never ever see a reason to implement 2 different ways to
>> traverse the elements, just to piss off people?
>>
>> If you make a PR that adds that to documentation, I will pull it if it
>> makes you feel better. I don't think it hurts, but don't think it's
>> worth my time to make such a PR.
>>
>> -Steve
>
> I can do PR for adding
>
> https://github.com/nordlow/justd/blob/master/range_ex.d#L527
>
> to Phobos.
>
> Were should I put it/them?

I think to be clear, the PR I said I will pull is for the documentation update. A doc change has no effect on the code.

I will certainly review one that adds iteration of both key and value (as that is pretty much a free addition given the existing code), but it will have to go through normal review process.

Note, the range you have referenced would not be accepted as it requires an unnecessary AA lookup for the value, and it requires access to phobos (this should be added to druntime). Take a look at object.di and see how the byKey and byValue ranges work. It would be trivial to add a byPair range (don't really like that name). The large controversy is regarding how it returns the "front" element.

It would have to satisfy 3 requirements I think:

1. I should be able to do foreach(k, v; r) on it.
2. I should be able to access both key and value separately for each element.
3. It cannot depend on phobos.

-Steve
December 16, 2014
On Tuesday, 16 December 2014 at 16:56:20 UTC, Steven Schveighoffer wrote:
>> I can do PR for adding
>>
>> https://github.com/nordlow/justd/blob/master/range_ex.d#L527
>>
>> to Phobos.
>>
>> Were should I put it/them?
>
> I think to be clear, the PR I said I will pull is for the documentation update. A doc change has no effect on the code.
>
> I will certainly review one that adds iteration of both key and value (as that is pretty much a free addition given the existing code), but it will have to go through normal review process.
>
> Note, the range you have referenced would not be accepted as it requires an unnecessary AA lookup for the value, and it requires access to phobos (this should be added to druntime). Take a look at object.di and see how the byKey and byValue ranges work. It would be trivial to add a byPair range (don't really like that name). The large controversy is regarding how it returns the "front" element.
>
> It would have to satisfy 3 requirements I think:
>
> 1. I should be able to do foreach(k, v; r) on it.
> 2. I should be able to access both key and value separately for each element.
> 3. It cannot depend on phobos.
>
> -Steve

Got it.
December 16, 2014
On Tue, Dec 16, 2014 at 11:56:20AM -0500, Steven Schveighoffer via Digitalmars-d-learn wrote:
> On 12/16/14 11:47 AM, "Nordlöw" wrote:
> >On Tuesday, 16 December 2014 at 16:08:09 UTC, Steven Schveighoffer wrote:
> >>I can never ever see a reason to implement 2 different ways to traverse the elements, just to piss off people?
> >>
> >>If you make a PR that adds that to documentation, I will pull it if it makes you feel better. I don't think it hurts, but don't think it's worth my time to make such a PR.
> >>
> >>-Steve
> >
> >I can do PR for adding
> >
> >https://github.com/nordlow/justd/blob/master/range_ex.d#L527
> >
> >to Phobos.
> >
> >Were should I put it/them?
> 
> I think to be clear, the PR I said I will pull is for the documentation update. A doc change has no effect on the code.
> 
> I will certainly review one that adds iteration of both key and value (as that is pretty much a free addition given the existing code), but it will have to go through normal review process.
> 
> Note, the range you have referenced would not be accepted as it requires an unnecessary AA lookup for the value, and it requires access to phobos (this should be added to druntime). Take a look at object.di and see how the byKey and byValue ranges work. It would be trivial to add a byPair range (don't really like that name). The large controversy is regarding how it returns the "front" element.
> 
> It would have to satisfy 3 requirements I think:
> 
> 1. I should be able to do foreach(k, v; r) on it.
> 2. I should be able to access both key and value separately for each
> element.
> 3. It cannot depend on phobos.
[...]

This has already been implemented, btw, but it was rejected because it did not use Phobos' Tuple type for .front. The code is here:

	https://github.com/D-Programming-Language/druntime/pull/574

One possibility is that we resurrect this PR and also add a new Phobos PR that wraps around .front to return Tuple instead (perhaps also rename byPair to something else in the process).


T

-- 
One Word to write them all, One Access to find them, One Excel to count them all, And thus to Windows bind them. -- Mike Champion
December 16, 2014
On Tuesday, 16 December 2014 at 17:47:18 UTC, H. S. Teoh via Digitalmars-d-learn wrote:
> also rename
> byPair to something else in the process).

What about by Python's byItem() and items()?
1 2 3
Next ›   Last »