[RFC] Add 'Sequence::isSorted'.

[mhermier]

While discussing for #899, it became apparent that #802 lacks test to code that a Sequence is sorted. Basically it means adding:

class Sequence {
  ...

  isSorted { ... }
  isSorted(comp) { ... }
}

[PureFox48]

I agree that an isSorted method would be useful though, given that the new sort method is in the List class, would it be better for isSorted to be a member of that class as well so that it has access to any sorting infrastructure it may need?

[mhermier]

A Sequence can be sorted despite the fact there is no sorting infrastructure, in particular I think about Range.

[PureFox48]

Yes, Range would require special treatment in #899 too as there's no point iterating through it when the elements are distinct by definition.

[ChayimFriedman2]

BTW, ranges can optimize more function, such as count.

[PureFox48]

Good point.

I'd expected that Range would override the count property inherited from Sequence in wren_core.c but I just had a look and couldn't find anything.

[ChayimFriedman2]

It does not. I'm working on a PR right now for that and some more.

Edit: Namely:

• Range.count.
• Range.contains(_).
• Range.skip(_).
• Range.take(_).
• Range.toList.
• List.toList.

[PureFox48]

Range.contains(_) will be particularly useful. I had a case today where I shied away from using it knowing that it would just iterate though the range to see if the value occurred.

[ChayimFriedman2]

Never optimize based on internal details, nor before you measured.

[PureFox48]

I agree with that maxim up to a point but, if I know something is bad and it's almost as easy to do something that isn't (a simple boolean expression here), I find it difficult to make my fingers type the former.

[ChayimFriedman2]

It's surprisingly hard to calculate the size of a range 😸.

[ChayimFriedman2]

@PureFox48 I don't know if you'd thought about that, but Range.contains() is not just min <= x <= max (or similar variations). It needs to preserve the original behavior, so it'll return true only if it the range contains this number exactly. For example, 1..10 doesn't contain 2.1, only 1.1..10 does.

@ruby0x1 Do it seems correct to you to break the current behavior and instead just make Range.contains(_) checks whether a value is included in the range? This will also simplify the implementation a lot.

[PureFox48]

Yes, I did know that range elements are not necessarily integral though it's not really a problem for me as my ranges almost invariably are integral and the value I'm checking for is also integral.

Rather than override Sequence.contains in a way that is not backwards compatible, perhaps a better idea would be to come up with a new method (includes or encloses maybe?) which does what we'd like it to do.

[ChayimFriedman2]

Maybe, but that's for another PR.

[mhermier]

Out of original topic but should be something like:

clamp(value, min, max) && (value - min) % step == 0 && (is_inclusive || value != max)

[ChayimFriedman2]

Already sent a PR: #901.