Option additions

[glennsl]

A few additions I've found useful. Of note:

• expect is inspired by Option.expect in Rust, to get more useful error messages.
• orElse was originally named as such because or was a reserved keyword. In rescript it no longer is, and orElse carries a slightly different meaning (lazy construction) for those coming from Rust. It therefore seems like a good idea to use or for this and reserve orElse to conform to, or at least not conflict with, the Rust convention.

[glennsl]

I'd also like to rename mapWithDefault and getWithDefault to mapOr and getOr because I think they're needlessly verbose as-is, but I know this would be a controversial change and has much wider impact so I've kept it out for now.

[zth]

So, the only thing we have left is deciding between expect and getExnWithMessage, then this is good to go.

[aspeddro]

I like expectExn

[zth]

Ok, let's do it like this:

Like this comment if you want it to be called expect.

[zth]

And this if you want it to be called getExnWithMessage.

[tsnobip]

And this if you want it to be called getExnWithMessage.

I think this name is more coherent with the other functions, could also be called getExnWithMsg to make it shorter.

[jmagaram]

Didn't see this until tonight. Please see https://github.com/rescript-association/rescript-core/issues/85

I've surveyed F#, OCaml, Belt, fp-ts and think there are a lot of holes to fill. I'm putting together a proposal.

[jmagaram]

Naming-wise I prefer flatten to flat. That was one on my tentative list of missing functions. I'll try to get my list together in next 24 hours so we can discuss.

[glennsl]

flat is named such because it's consistent with Array.flat from JavaScript, and sticking close to JS APIs is an objective for this project. Otherwise I agree :)

[zth]

Didn't see this until tonight. Please see #85

I've surveyed F#, OCaml, Belt, fp-ts and think there are a lot of holes to fill. I'm putting together a proposal.

Please beware it's not a goal for this project to cover that ground. That's better done in another standalone library.

[jmagaram]

I’m confused about the goal. Cover what ground? F# doesn’t go crazy with their standard library and I think we’re missing some basic commonly used things. Fp -ts is a bit much - don’t understand a lot of the functional type classes. How about you see what I propose and let me know if it is too much.

---

From: Gabriel Nordeborn @.> Sent: Tuesday, March 7, 2023 4:10:59 AM To: rescript-association/rescript-core @.> Cc: jmagaram @.>; Comment @.> Subject: Re: [rescript-association/rescript-core] Option additions (PR #57)

Didn't see this until tonight. Please see #85https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Frescript-association%2Frescript-core%2Fissues%2F85&data=05%7C01%7C%7C89ebbb323a984fc5cf5608db1f0503dd%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638137878620563032%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=92hFRNszPvWe%2F2n6q5OMLMMcOwo7Qgs%2F%2FBgwq15ZdhE%3D&reserved=0

I've surveyed F#, OCaml, Belt, fp-ts and think there are a lot of holes to fill. I'm putting together a proposal.

Please beware it's not a goal for this project to cover that ground. That's better done in another standalone library.

— Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Frescript-association%2Frescript-core%2Fpull%2F57%23issuecomment-1458063750&data=05%7C01%7C%7C89ebbb323a984fc5cf5608db1f0503dd%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638137878620563032%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2BFf0l7Kq2jggfHMWFyS4wUU1nDFEmYI9grlZ6YCxC2I%3D&reserved=0, or unsubscribehttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FADLZYS3LVQ5XFIOZTCC7TWTW24QVHANCNFSM6AAAAAAVBAPU4Y&data=05%7C01%7C%7C89ebbb323a984fc5cf5608db1f0503dd%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638137878620563032%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=UYS3oOiJBv3EiGw5lfUC38%2FdX4XfnyXZ9Mu4qjENaF4%3D&reserved=0. You are receiving this because you commented.Message ID: @.***>

[jmagaram]

Small thing - when I switched from JavaScript I was happy to see the word "keep" instead of "filter" because it made it super clear what's happening. I still find it a bit confusing whether I am filtering stuff IN or OUT. Shorter word too. I'm not going to fight that battle though. "filter" is the word used almost everywhere and getting people over from JavaScript is the priority.

[jmagaram]

Ok see the issue for lots of ideas... https://github.com/rescript-association/rescript-core/issues/85

[jmagaram]

And this if you want it to be called getExnWithMessage.

I think this name is more coherent with the other functions, could also be called getExnWithMsg to make it shorter.

I don't know if we need these "withMessage" or "expect" features, regardless of what they are called. But I'm curious why we don't use optional labeled parameters instead so we don't pollute the namespace with tons of overrides. let getExn: (t<'a>, ~message: string=?) => 'a.

[glennsl]

Optional labeled parameters cannot be put at the end, due to currying. If on function application ~message is omitted, it's not clear whether the intent is to fully apply it, or if it should return a function to later apply the missing ~message argument.

[jmagaram]

Forgive me if I'm not understanding, but why not...

 let getExn: (~message: string=?, t<'a>) => 'a

I just tried it as both these flavors it returns an int, no warnings.

let x = Some(4)->getExn
let y = Some(4)->getExn(~message="wow!")

[glennsl]

Sure, that's a good alternative, though it is a bit more verbose. This style also won't work with bindings. This isn't a binding, of course, but if we're aiming for a consistent style that does work with bindings then this isn't it.

Another minor benefit of the expect name is that it sort of guides the user to a certain style of message that tends to yield more information to the developer trying to figure out why it happened. See the Rust documentation for example.

[jmagaram]

The first impression of @cknitt was this was for testing. Me too. It sounds very much like assert. I do not expect a name like expect to return a value. Those links @zth passed around used a message parameter on getExn so that means to me people think it is a natural place to put it. I prefer getExnMessage or getExnMsg Not trying to beat a dead horse here but wanted to speak my truth before this goes live.

[zth]

Ok, adding an optionally labelled argument might actually be the best bet here in my opinion (let getExn: (~message:string =?, option<'a>) => 'a. One problem I see with that is that it will break existing code that does:

someOptArray->Array.map(Option.getExn)

...but. One goal of Core is to be "uncurried by default"-ready, and I believe that the above code would work as is in uncurried by default (cc @cristianoc).

@glennsl @cknitt what do you think?

[cristianoc]

Ok, adding an optionally labelled argument might actually be the best bet here in my opinion (let getExn: (~message:string =?, option<'a>) => 'a. One problem I see with that is that it will break existing code that does:

someOptArray->Array.map(Option.getExn)

...but. One goal of Core is to be "uncurried by default"-ready, and I believe that the above code would work as is in uncurried by default (cc @cristianoc).

@glennsl @cknitt what do you think?

The problem is that this is not t-first. In uncurried, one can have a final optional argument without unit (starting from v11). So the API is not expressible until v11 is out.

We should design with v11 in mind, as that changes the possible tradeoffs. This also means that adding certain functions will have to wait.

[cristianoc]

Relevant for option design too: https://github.com/rescript-lang/rescript-compiler/issues/5628 Depending on whether new syntax is introduced, more patterns could be expressible natively.

[glennsl]

Ok, adding an optionally labelled argument might actually be the best bet here in my opinion (let getExn: (~message:string =?, option<'a>) => 'a. One problem I see with that is that it will break existing code that does:

someOptArray->Array.map(Option.getExn)

...but. One goal of Core is to be "uncurried by default"-ready, and I believe that the above code would work as is in uncurried by default (cc @cristianoc).

@glennsl @cknitt what do you think?

I think it's fine. But I still think expect is better, because it's shorter and more explicit about intent. And it has precedent in Rust, where as far as I know no-one has confused production code for being unit tests when encountering it.

[jmagaram]

Our pattern is to group the getters together getUnsafe, getOr, getExn and I think we should stick to that pattern. I don't agree the intent is clearer calling it expect because it breaks the pattern and because the term is similar to assert. To me it is weird that Rust does what it does because they group things with unwrap.... They did originally have an unwrap_expect! I also thought part of our pattern was using Exn to clearly indicate when an exception can be thrown.

https://stackoverflow.com/questions/66362625/why-is-rusts-expect-called-expect

Eventually I want the Option module to have lots of functions inspired by other languages - Rust even as Option.xor. I don't think we need to worry about too many functions polluting the Intellisense menu and we don't need to hide this feature inside an optional parameter if it is useful. I'm happy to have this feature. I think it is less important than things like concat, reduce, exists, map2.

If we had a labeled optional argument at the end, must the label be used after the curry change? Because then we're still adding ~message=... or ~msg=... and it gets verbose. If not this seems like the best way to go - just type a custom message if you want one.

If someone really want this feature now here are some naming ideas and you can see how it fits in.

  let getOr: (t<'a>, 'a) => 'a
  let getOrWith: (t<'a>, unit => 'a) => 'a // proposed lazy form
  let getUnsafe: t<'a> => 'a

  let getExn: t<'a> => 'a

  let getExnWith: (t<'a>, string) => 'a
  let getExnWithMessage: (t<'a>, string) => 'a
  let getExnWithMsg: (t<'a>, string) => 'a
  let getExnMsg: (t<'a>, string) => 'a
  let getExnMessage: (t<'a>, string) => 'a
  let getOrMessageExn: (t<'a>, string) => 'a
  let getOrMsgExn: (t<'a>, string) => 'a
  let expect: (t<'a>, string) => 'a
  let expectExn: (t<'a>, string) => 'a