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.

[cknitt]

@glennsl Thanks for all the work you are doing!

[cknitt]

Not sure if we shouldn't rather leave the implementation of such a function to e.g. testing frameworks (which might also want to raise a different exception).

[glennsl]

This isn't for testing. You should consider using this every time you reach for getExn because it gives you a better, easier to track down error message when the assumption that lead you to using it inevitably fails in production 🙃

[zth]

One thing that comes to mind here - do we need an additional alternative where you can pass your own exception? I recall people in the community talking about adding that themselves.

[glennsl]

What's the use case for that? To have a distinct exception that they can catch?

[zth]

Yeah, exactly. But let's ignore it for now, feels peripheral.

[cknitt]

Ok, right, so there is definitely demand in the community for something like this.

And what do you think about the naming? Usually we have exn in the method name when the function might raise. That wouldn't be the case with expect. Maybe it should be getExnWithMessage?

[glennsl]

I think that would ensure that absolutely no-one would pick this over getExn, unfortunately 😉

[glennsl]

expectExn could be an alternative. Though I don't think the convention is that functions that raise should always use Exn, but rather that it signifies a variant of a function that distinguishes itself by raising. Similar to how not all functions that can return an option uses Opt.

Is there not an annotation that can be put on functions that raise? I thought reanalyze utilized that, but I can't find it now.

[zth]

Naming this one is hard for sure... I'm trying to come up with a good alternative as well. I like expect because I've used it a lot in Rust, but I'm not sure it'd feel very familiar to people who haven't used Rust. I'm ok with getExnWithMessage, but I understand if it feels too verbose. So let's maybe think a round or two more about what could be a good alternative name, before we decide.

@glennsl correct, it's @raises: https://rescript-lang.org/syntax-lookup#raises-decorator
We should eventually work our way through all the bindings and ensure all of them have @raises where appropriate. That'll be a lot of work though, so probably wise to do it little by little, over time. It's also less important than the general documentation is right now.

[zth]

I logged an issue for @raises.

Still haven't figured out a better name than expect or getExnWithMessage. Anyone else got something? Otherwise we'll need to figure out the best choice between those two.

[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 #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 #85<https://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 GitHub<https://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 unsubscribe<https://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... #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: rescript-lang/rescript#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