I'm not sure the semantics -- but its always been cumbersome to manually check ranges. A List.getSafeRange would be nice. Maybe to keep it simple always return null if either start or end is out of bounds?
brianFri 29 May 2020
I like it, but think the semantics should be to clip which is how we did it in Axon. For example:
"abcd".getSafe(1..10)
clip option would returns "bcd"
null option would return null
The clip option seems more generally useful. Although maybe we call that one clip?
andySun 31 May 2020
Eh I think if you passed in an explicit range it needs to "fail" with null. Otherwise you need to check the range anyways so doesn't save you any code.
But I'm not opposed to a "clipSafe" method either. Those are probably both valid use cases.
brianWed 1 Jul 2020
Any other comments?
Option 1: add getSafeRange which returns null when out of range
Option 2: add getSafeRange which returns clipped range
Option 3: add getClippedRange which return clipped range
Option 4: option 1 and 3 combined (two new methods)
Option 5: add getSafeRange with a flag to determine null/clip behavior
And whatever we add would also need to be added to Str too
andyWed 1 Jul 2020
Option #4
SlimerDudeMon 13 Jul 2020
It opens up a lot of points already raised in List.getSafe but what of a default checked := true parameter?
It saves on polluting List with a lesser used method, it's consistent with a lot of other Fantom APIs, and is just as useful.
str := "abcd"
str.getRange(1..10, false) // return null
str.getRange(1..10, false) ?: str[1..-1] // return a default value
Returning null isn't such a hardship because you can always use elvis to set a default value.
Disregarding the method name, I guess I'm opting for Option 1!
brianFri 29 Jan 2021
Ticket promoted to #2795 and assigned to brian
Not sure we really had a resolution on new APIs. I don't really love adding two methods which are so close. So maybe we just stick with null and not deal with clipping problem now:
A completely separate issue - I find using List.getSafe with negative indices is a huge pain. Pretty much 100% of the time I want -1 to always result in null instead of the last item. Because of that boundary condition I almost end up having to add an extra check that negates the getSafe conciseness.
Changing that would be a breaking change, but I suspect is what almost all code is expecting anyways. Any opposition to that change?
andyFri 29 Jan 2021
I would vote just the one getRangeSafe for now
I find using List.getSafe with negative indices is a huge pain. Pretty much 100% of the time I want -1 to always result in null instead of the last item.
That seems like a dangerous gotcha :) A quick look at my code and doesn't seem to turn up too much -- but seems like we should manage that change if we make over a transition period to be safe.
SlimerDudeFri 29 Jan 2021
I suspect this ticket is largely aimed at Andy, but I'm happy with a nullable method (sure, add another one to the List!).
As for changing the runtime behaviour of the core List.getSafe() method ... that sounds very dangerous and a particularly nefarious breaking change - because you'll never know if you're affected until it's too late. SkySpark is growing a community of Fantom users, and who knows how it will adversely affect them?
I (also) don't like the inconsistency that -1 will sometimes do this, and sometimes do that.
I've seen plenty of other proposals on this forum approved in principle but rejected on the basis of breaking compatibility. Perhaps then there is cause for a new'er Fantom 1.1 codebase that incorporates all these breaking changes?
brianFri 7 May 2021
I started this work, and updated the test suite to plug in new tests.
However, I don't get a warm fuzzy adding this to the core. Its actually not just a few lines of new code; because the existing getRange routes to utility methods in Range, it will require quite a bit of new code and new utilities (unless we just implement it with a try block).
Plus looking thru my own code, I am not sure how often I could actually use it. I still almost always need checks for the negative index situation. Countless times I've done something like this a try block, only to have the code fail a different way because of negative index support.
Can we get some actual use cases and thoughts on it?
andy Fri 22 May 2020
I'm not sure the semantics -- but its always been cumbersome to manually check ranges. A
List.getSafeRange
would be nice. Maybe to keep it simple always returnnull
if either start or end is out of bounds?brian Fri 29 May 2020
I like it, but think the semantics should be to clip which is how we did it in Axon. For example:
The clip option seems more generally useful. Although maybe we call that one clip?
andy Sun 31 May 2020
Eh I think if you passed in an explicit range it needs to "fail" with
null
. Otherwise you need to check the range anyways so doesn't save you any code.But I'm not opposed to a "clipSafe" method either. Those are probably both valid use cases.
brian Wed 1 Jul 2020
Any other comments?
Option 1: add getSafeRange which returns null when out of range
Option 2: add getSafeRange which returns clipped range
Option 3: add getClippedRange which return clipped range
Option 4: option 1 and 3 combined (two new methods)
Option 5: add getSafeRange with a flag to determine null/clip behavior
And whatever we add would also need to be added to Str too
andy Wed 1 Jul 2020
Option #4
SlimerDude Mon 13 Jul 2020
It opens up a lot of points already raised in List.getSafe but what of a default
checked := true
parameter?It saves on polluting List with a lesser used method, it's consistent with a lot of other Fantom APIs, and is just as useful.
Returning
null
isn't such a hardship because you can always use elvis to set a default value.Disregarding the method name, I guess I'm opting for Option 1!
brian Fri 29 Jan 2021
Ticket promoted to #2795 and assigned to brian
Not sure we really had a resolution on new APIs. I don't really love adding two methods which are so close. So maybe we just stick with null and not deal with clipping problem now:
A completely separate issue - I find using List.getSafe with negative indices is a huge pain. Pretty much 100% of the time I want -1 to always result in null instead of the last item. Because of that boundary condition I almost end up having to add an extra check that negates the getSafe conciseness.
Changing that would be a breaking change, but I suspect is what almost all code is expecting anyways. Any opposition to that change?
andy Fri 29 Jan 2021
I would vote just the one
getRangeSafe
for nowThat seems like a dangerous gotcha :) A quick look at my code and doesn't seem to turn up too much -- but seems like we should manage that change if we make over a transition period to be safe.
SlimerDude Fri 29 Jan 2021
I suspect this ticket is largely aimed at Andy, but I'm happy with a nullable method (sure, add another one to the
List
!).As for changing the runtime behaviour of the core
List.getSafe()
method ... that sounds very dangerous and a particularly nefarious breaking change - because you'll never know if you're affected until it's too late. SkySpark is growing a community of Fantom users, and who knows how it will adversely affect them?I (also) don't like the inconsistency that
-1
will sometimes do this, and sometimes do that.I've seen plenty of other proposals on this forum approved in principle but rejected on the basis of breaking compatibility. Perhaps then there is cause for a new'er Fantom 1.1 codebase that incorporates all these breaking changes?
brian Fri 7 May 2021
I started this work, and updated the test suite to plug in new tests.
However, I don't get a warm fuzzy adding this to the core. Its actually not just a few lines of new code; because the existing getRange routes to utility methods in Range, it will require quite a bit of new code and new utilities (unless we just implement it with a try block).
Plus looking thru my own code, I am not sure how often I could actually use it. I still almost always need checks for the negative index situation. Countless times I've done something like this a try block, only to have the code fail a different way because of negative index support.
Can we get some actual use cases and thoughts on it?