refactor: WithDefault -> Or

[glennsl]

Renames mapWithDefault tomapOr and getWithDefault to getOr across all modules.

I expect this to be a bit controversial, so I suggest we do a vote on this. 👍 if you're for the change, 👎 if you're against.

[zth]

Doesn't seem very controversial so far 😄

My thoughts:

• The "WithStuff" pattern is fairly prevalent in the code base already. Would changing this be a one off fix, or will it make the rest of the functions named that way feel inconsistent?
• For the functions people might already be using (Option primarily), can we keep the WithDefault functions around as aliases, but deprecate using them and refer to the new Or methods?
• If we do this change, I'd like to wait with doing it until we've gotten another release out, and are "feature complete" as for the most important docstrings etc. That's currently what'd improve the experience of using Core the most.

I'm for doing these types of changes now that we're so early in the life of Core. But I'm way of churn. On the other hand, this is probably the best time to do it, before Core has spread more.

[jmagaram]

I like the shorter terminology. There is a related issue - lazy. Whenever we have a default I think there should be a flavor with a lazy default. F# does this. F# uses with suffix for lazy. Here's what the Option module might look like. It starts to get out of control with the U uncurried flavors.

module type Option = {
  type t<'a>
  let get: t<'a> => option<'a>
  let getUnsafe: t<'a> => 'a
  let getExn: t<'a> => 'a
  let getOr: (t<'a>, 'a) => 'a
  let getOrWith: (t<'a>, unit => 'a) => 'a
  let orElse: (t<'a>, option<'a>) => option<'a>
  let orElseWith: (t<'a>, unit => option<'a>) => option<'a>
  let map: (t<'a>, 'a => 'b) => option<'b>
  let mapOr: (t<'a>, 'b, 'a => 'b) => 'b
  let mapOrWith: (t<'a>, unit => 'b, 'a => 'b) => 'b
}

[glennsl]

Doesn't seem very controversial so far smile

😄

The "WithStuff" pattern is fairly prevalent in the code base already. Would changing this be a one off fix, or will it make the rest of the functions named that way feel inconsistent?

I don't think it does. In my mind WithXXX is a fallback rule to use when there's no better choice. I don't think this level of consistency is very important, or that consistency in itself is much of an argument. If it was, we'd have a lot of really clunky function names I think. Instead of map2, for example, we might have mapWithOther. Instead of indexOfFrom we might have indexOfWithStartingIndex. Instead of slice we might have sliceWithStartAndEndIndex. I don't think we really want that.

As I've said elsewhere, consistency is a means to an end. It's a way to make it easy to remember different variants of the same function, and to easily see what's available and their differences when auto-completing. And I'm sure other ends as well. But it's not an end in and of itself. And it has significant drawbacks, such as being overly verbose and impacting readability by increasing the amount of noise.

For the functions people might already be using (Option primarily), can we keep the WithDefault functions around as aliases, but deprecate using them and refer to the new Or methods?

Certainly! (Edit: Done!)

If we do this change, I'd like to wait with doing it until we've gotten another release out, and are "feature complete" as for the most important docstrings etc. That's currently what'd improve the experience of using Core the most.

I'm not sure I understand the reasoning here, but I'm not opposed to it.

I'm not personally using this yet, since it doesn't seem ready and stable enough. And I'm in no rush. If I was I'd use it via the same kind of shims I'm using between the Belt and Js APIs to make them consistent and complete. This is starting to get really close though!

[zth]

Time to pick this up again and push it over the finish line @glennsl ?

[glennsl]

Rebased!

[zth]

Sorry @glennsl, could I get one more rebase? 🙏

[glennsl]

Done.