www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Proposal to deprecate "retro.source"

reply "monarch_dodra" <monarchdodra gmail.com> writes:
I just got snagged in a "funny" bug.

Using a generic template, I wanted a which shrank from the back. 
To do this, I made a call to "find(r.retro, stuff).source"

The idea being: Search from the back, and then extract the 
forward-iteration range from the result of find.

I got greeted with an "unknown property: source": WTF?

Turns out that in my function, "r" was already a retro range. In 
this condition, retro(r) un-retroed my range, which was all fine 
and well, but now it didn't have "source" anymore :/

The solution is actually quite easy, and already implemented: 
Just retro again!

The problem though is in the way the documentation "The original 
range can be accessed by using the source property" and "Applying 
retro twice to the same range yields the original range": Looks 
like we forgot to notice these two sentences are contradicting.

--------
To this end, I propose to deprecate "retro.source", and simply 
reword as:
*"Applying retro twice to the same range yields the original 
range"
*"The original range can be accessed by applying retro"

--------
I'd also like to not that this would avoid some *incredibly* 
unsafe code, that checks if R is of type retro by simply checking 
the *source* field... eg: to check if R1 is the retro of R2:
"is (typeof(r1.source) == typeof(r2))" //BAD! Could also be take, 
or who knows what!
which could be replace by
"is (typeof(r1.retro()) == typeof(r2))" //Good: Safe.

Thoughts?

(or if not deprecate, at least reword the documentation...)
Nov 06 2012
next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Tuesday, November 06, 2012 18:05:27 monarch_dodra wrote:
 Thoughts?

We need to figure out how we want to handle source in general. There are some cases where it's really useful to be able to get at the original range - the prime example of this being functions like remove in std.container. The problem is that in most (all?) cases, you can't really use source without understanding how the wrapper range works, because the source doesn't necessarily have the same number or order of elements as the wrapper (e.g. if filter had source, the elements that source had could be very different from what the filtered range had, or in the case of retro, the order is completely different). In some cases, like retro and take, it's quite easy for the function accessing source to know how to handle it properly. In others, it gets far more complicated. Andrei has expressed a desire to expose source in general. I don't know how wise that is or isn't, but it's something that needs to be explored. In the case of retro, I think that it would good to have source exposed for std.container's use. It's easy for std.container to understand what retro's supposed to do, and use it accordingly, and I think that it would be silly for it have to call retro on the retroed range to do that. I do agree however that in general, it doesn't make sense to access source. As for
 The problem though is in the way the documentation "The original 
 range can be accessed by using the source property" and "Applying 
 retro twice to the same range yields the original range": Looks 
 like we forgot to notice these two sentences are contradicting.

I don't see anything contradictory at all. If you call retro on a retroed range, you get the original. If you access source, you get the original. Where's the contradiction? In both cases, you get the original range. So, I see no reason to deprecate retro's source. - Jonathan M Davis
Nov 08 2012
prev sibling next sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Thursday, 8 November 2012 at 09:18:54 UTC, Jonathan M Davis 
wrote:
 In the case of retro, I think that it would good to have source 
 exposed for
 std.container's use. It's easy for std.container to understand 
 what retro's
 supposed to do, and use it accordingly, and I think that it 
 would be silly for
 it have to call retro on the retroed range to do that. I do 
 agree however that
 in general, it doesn't make sense to access source.

Yes, accessing the original range is useful, but AFAIK, container doesn't use retro in any way. It does it with take (which also has a source field). For take, there is no way to extract the source other than with a specialized function. regarding retro, I find it silly it has a "source" field at all, when the original could just be retrieved using retro again (and just as efficiently). I don't see any way using source over retro could be useful to anyone at all, except for actually implementing retro().retro() itself (in which case a _source would have done just as well).
 As for

 The problem though is in the way the documentation "The 
 original range can be accessed by using the source property" 
 and "Applying retro twice to the same range yields the 
 original range": Looks like we forgot to notice these two 
 sentences are contradicting.

I don't see anything contradictory at all. If you call retro on a retroed range, you get the original. If you access source, you get the original. Where's the contradiction?

In "If you access source, you get the original". This is only true if the "The original" is not itslef already a retro. This mind sound silly, but you may not have control of this. For example, For example, imagine you want a find function that searches from the end, and returns the result of everything before the match: //---- import std.stdio; import std.algorithm; import std.range; auto findBefore(R)(R r, int n) { auto a = r.retro(); auto result = a.find(n); static assert(is(typeof(result) == typeof(r.retro()))); //result is a retro return result.source; //Error: undefined identifier 'source' } void main() { auto a = [0, 1, 2, 3, 4, 5]; a.findBefore(3).writeln(); //expect [1, 2, 3] a.retro().findBefore(3).writeln(); //expect [5, 4, 3] auto b = indexed([2, 1, 3, 0, 4, 5], [3, 1, 0, 2, 4, 5]); assert(b.equal(a)); // b is [0, 1, 2, 3, 4, 5] b.retro().findBefore(3).writeln(); // produces [2, 1, 3, 0, 4, 5]... } //---- In "findBefore3", one should expect getting back the "original range", but that is not the case. However, using "return result.retro()"; works quite well. Things get even stranger if the underlying range also as a "source" filed itself (because of the auto return type). Both the issues can be fixed with "return result.source;" -------- I don't think there is anything to be gained using "source". I don't think it would be worth deprecating it either, and it is not a huge issue, but I think the documentation should favor the use of double retro over source, or even mention source at all. Less surprises is always better.
Nov 08 2012
prev sibling next sibling parent "Mehrdad" <wfunction hotmail.com> writes:
Isn't this easy to fix?

Just make sure typeof(retro(r)) == typeof(retro(retro(retro(r))))

so that you always get back a retro'd range.
Nov 08 2012
prev sibling next sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Thursday, November 08, 2012 10:56:38 monarch_dodra wrote:
 On Thursday, 8 November 2012 at 09:18:54 UTC, Jonathan M Davis
 
 wrote:
 In the case of retro, I think that it would good to have source
 exposed for
 std.container's use. It's easy for std.container to understand
 what retro's
 supposed to do, and use it accordingly, and I think that it
 would be silly for
 it have to call retro on the retroed range to do that. I do
 agree however that
 in general, it doesn't make sense to access source.

Yes, accessing the original range is useful, but AFAIK, container doesn't use retro in any way. It does it with take (which also has a source field). For take, there is no way to extract the source other than with a specialized function.

std.container doesn't use retro right now, but it really should (though that would require externalizing retro's return type). For instance, what would you do if you had to remove the last five elemets from a DList? You can't simply take the last 5 and pass that to remove with something like take(retro(list[], 5)) or retro(take(retro(list[], 5))), because the resulting type is unrecognized by remove. You're forced to do something like list.remove(popFrontN(list[], walkLength(list[]) - 5)); which is highly inefficient.
 regarding retro, I find it silly it has a "source" field at all,
 when the original could just be retrieved using retro again (and
 just as efficiently). I don't see any way using source over retro
 could be useful to anyone at all, except for actually
 implementing retro().retro() itself (in which case a _source
 would have done just as well).

Andrei has expressed interest in having _all_ ranges (or at least a sizeable number of them) expose source. That being the case, using retro to get at the original doesn't really make sense. That's what source is for. retro naturally returns the original type when you retro it again, because it avoids type proliferation, but that's specific to retro. Now, how useful source will ultimately be in generic code which doesn't know exactly what range type it's dealing with, I don't know. It may ultimately be pretty much useless. But even if you have to know what the type is to use it appopriately, since they'd generally be exposing the original range through source, it makes sense that retro would do the same.
 As for
 
 The problem though is in the way the documentation "The
 original range can be accessed by using the source property"
 and "Applying retro twice to the same range yields the
 original range": Looks like we forgot to notice these two
 sentences are contradicting.

I don't see anything contradictory at all. If you call retro on a retroed range, you get the original. If you access source, you get the original. Where's the contradiction?

In "If you access source, you get the original". This is only true if the "The original" is not itslef already a retro. This mind sound silly, but you may not have control of this. For example,

It makes perfect sense that retro would return the same type when retro is called on an already retroed range. And that's the _only_ case where source wouldn't exist. The real problem with relying on source from the type returned by retro is the fact the result could be _another_ range which exposes source rather than retro - e.g. retro(retro(take(range, 5))). The docs aren't really incorrect in either case. It's just that in that one case, the result is a different type than the rest. I have no problem with removing mention of source from the docs. I don't think that it should be used normally. I do think that it makes sense to have it, but it makes sense primarily as part of the general approach of giving ranges source members. Certainly, using source directly after a call to retro makes no sense. For using source to _ever_ make sense (in general, not just with retro), you need to know that you're dealing with a wrapper range. So, using source immediately after having called a function which potentially returns a wrapper range doesn't really make sense. It makes sense when you already know that you're dealing with a wrapper range (e.g. you already know that it's a Take!Whatever), and even then, I think that it primarily makes sense when you're looking for a specific type that's being wrapped (as is the case with std.container) rather than when dealing with a generic type which was wrapped. - Jonathan M Davis
Nov 08 2012
prev sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Thursday, 8 November 2012 at 18:20:31 UTC, Jonathan M Davis 
wrote:
 On Thursday, November 08, 2012 10:56:38 monarch_dodra wrote:
 On Thursday, 8 November 2012 at 09:18:54 UTC, Jonathan M Davis
 
 wrote:
 In the case of retro, I think that it would good to have 
 source
 exposed for
 std.container's use. It's easy for std.container to 
 understand
 what retro's
 supposed to do, and use it accordingly, and I think that it
 would be silly for
 it have to call retro on the retroed range to do that. I do
 agree however that
 in general, it doesn't make sense to access source.

Yes, accessing the original range is useful, but AFAIK, container doesn't use retro in any way. It does it with take (which also has a source field). For take, there is no way to extract the source other than with a specialized function.

std.container doesn't use retro right now, but it really should (though that would require externalizing retro's return type). For instance, what would you do if you had to remove the last five elemets from a DList? You can't simply take the last 5 and pass that to remove with something like take(retro(list[], 5)) or retro(take(retro(list[], 5))), because the resulting type is unrecognized by remove. You're forced to do something like list.remove(popFrontN(list[], walkLength(list[]) - 5)); which is highly inefficient.

It's funny you bring this up, because I've been wrapping my head around this very problem for the last week. The root of the problem is that Bidirectional ranges (as *convenient* as they are) just don't have as much functionality as iterators or cursors. If you've used DList more than 5 minutes, you know what I'm talking about. The "retro" trick you mention could indeed be a convenient mechanic. (Keep in mind that take is forward range, so can't be retro'ed though). I'm just afraid of what it would really mean to interface with a retro'ed range: What exactly does that range represent for the original container? What we'd *really* need (IMO) is a takeBack!Range, that would only implement back/popBack. No, the resulting range wouldn't *actually* be a range (since it wouldn't have a front), but it would still be incredibly useful even if *just* to interface with containers, eg: list.remove(list[].takeBack(5)); //Fine, that's not a "real" range, but I know how to interface with that. I'll toy around with this in my CDList implementation. ---- PS: The problem you bring up is common to all bidirRanges, and not just containers: As a general rule, there is no way to extract a subrange from the end of a bidirrange...
 [SNIP]*The rest*[/SNIP]

 - Jonathan M Davis

Read and acknowledged.
Nov 08 2012