unisonweb / base

Unison base libraries
https://share.unison-lang.org/@unison/base
18 stars 6 forks source link

Unintuitive and inconsistent behavior in range functions #169

Open ceedubs opened 1 year ago

ceedubs commented 1 year ago

Problem/background

Many of the functions related to ranges in base are exclusive on the upper bound, which always makes me think a bit harder, and I suspect that it would be surprising to newcomers. For example, I think that most people looking at the code Random.natIn 1 2 would expect it to generate 1 or 2 with equal probability, but in fact it will always generate 1.

The exclusive upper bound is also problematic when you want your range to include the max value (example: maxNat). I don't think that there's any simple way to achieve this functionality with exclusive ranges.

There are also some inconsistencies in base. For example Random.natIn lower upper is exclusive in upper but Text.patterns.charRange is inclusive in upper.

Proposed solution

I think that ranges should generally be inclusive and that exclusive ranges should be explicit in the name. So Random.natIn lower upper would include upper and Random.nat.range.closed lower upperExcluded (or whatever the name would be) would exclude upperExcluded.

pchiusano commented 1 year ago

@ceedubs exclusive upper bound for int ranges is usually what's done in languages with 0-based indexing. So the valid indices of a list of size n is List.range 0 n and not List.range 0 (n - 1).

If we were to change this, I like the idea of offering both, and to help with change management, do several releases where there is no default and you have to write natIn.closed or natIn.open or whatever.

ceedubs commented 1 year ago

I feel like the exclusive upper bound for index-based access is kind of carry-over from C-like languages and maybe is just confusing for a language like Unison where you typically are using higher-level functions like map and foldLeft. But I also wouldn't want such a change to trip up people who are used to exclusive bounds from other languages. Personally I'd be okay with always requiring an explicit inclusive/exclusive or closed/open.

Another option that I'm not sure that I like is a helper Range a type that can be either closed or open that range-like functions take as an argument.

My list of priorities would be:

  1. Make sure that the existing APIs are consistent.
  2. Define a convention for open/closed ranges (whether one is "default" or both are explicit).
  3. Make sure that it's possible to include max values in ranges.
runarorama commented 1 year ago

I agree that for newcomers, inclusive ranges make the most intuitive sense and are least surprising. But in almost every other language (notably both Python and JavaScript) ranges that are inclusive of the lower bound and exclusive of the upper bound are the norm.

I have a little library with an Interval type that could be integrated into Base that would make this kind of thing explicit:

https://share.unison-lang.org/@runarorama/intervals/code/releases/1.0.3/latest/types/Interval

ceedubs commented 1 year ago

@runarorama ooh I like how explicit your Interval library makes things. Some of the infix symbols for creating them might be a little controversial, but generally it looks like a simple and small library that I could see merging into base and using there.

timjs commented 1 year ago

The library inderdaad looks nice @runarorama!

About the operators: is it syntactically possible in Unison to use .., >.., >..<, and ..< as operator identifiers? This would make them less controversial and more like those used in Swift and Haskell for example.

runarorama commented 1 year ago

Names with dots in them are possible, but problematic as it's also used as a namespace separator