Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider name change for OptionT<T> #49

Closed
iwburns opened this issue Jul 30, 2018 · 6 comments
Closed

Consider name change for OptionT<T> #49

iwburns opened this issue Jul 30, 2018 · 6 comments

Comments

@iwburns
Copy link
Owner

iwburns commented Jul 30, 2018

I'd like to consider changing OptionT<T> to something less "bad" - for the lack of a better term - before we go 1.0. This is partly because it forces Result<T, E> to be ResultT<T, E> for consistency purposes, but also because I just don't like OptionT<T> at all.

In addition, if we were to move forward with #40 (which I'd really like to do in some form or fashion, but maybe not until after 1.0). I'd hate to be forced into naming it ValidationT<T, E> just to match OptionT<T> and ResultT<T, E>.

My only good suggestion at this point is Optional<T> (keeping Some<T> and None), but we could also switch to the other naming convention which is Maybe<T> with Just<T> and Nothing as the two variants. I'm not as fond of the Maybe naming convention, but if you guys think that's better than crow-barring our way through this problem just to use a variant of the word "Option", then I could be convinced to change it.

Do you guys have any thoughts on this?
cc @brycehipp @ryanguill

@ryanguill
Copy link
Collaborator

I think changing it to Optional is fine. I don't think we should switch to Maybe with the different api - there are subtle differences and I don't see any benefit in changing, especially at this point.

Why did we go with OptionT initially?

@iwburns
Copy link
Owner Author

iwburns commented Jul 31, 2018

Why did we go with OptionT initially?

I wanted this library to be something that could be used in the browser as well as other contexts, but sadly Option is already a global in the browser. This doesn't completely prohibit us from using Option, but since most of the time you'd want to import Option/Result directly (so you could write Option.some(1)) instead of just importing nullshield as a whole (and having to write nullshield.Option.some(1)) it'd be pretty annoying to be forced into the latter in the browser.

I couldn't think of a good way around this at the time, so I just stuck a T on the end of the name and went with it. Thinking back now Optional is a much better option that I either simply didn't think of or rejected for some unknown reason.

However, if you can think of a better solution to this problem, I'm all ears.

@iwburns
Copy link
Owner Author

iwburns commented Aug 2, 2018

So, summing up here, just to be clear about the result (heh) of making this change:

Before After
OptionT<T> Optional<T>
ResultT<T, E> Result<T, E>
ValidationT<T, E>* Validation<T, E>*

*(assuming we decide to build this one)

Seem good to you guys?

I'm slightly annoyed by the fact that Optional is now an adjective and they both used to be nouns. I would have liked for it to all be symmetrical in that aspect, but I guess it's not a big deal.

@ryanguill
Copy link
Collaborator

LGTM. Think of them as containers and all of them are adjectives. Optional Container, Result Container, Validation Container.

@brycehipp
Copy link
Collaborator

In total agreement of the name change.

Thinking of them as containers definitely helps wrap my mind around them too.

@iwburns
Copy link
Owner Author

iwburns commented Aug 7, 2018

Awesome! I'll get to work on this asap. Thanks for the feedback here guys!

This was referenced Aug 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants