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

Prefix partial function names with `unsafe` #42

Closed
robotlolita opened this Issue Sep 26, 2016 · 1 comment

Comments

Projects
None yet
2 participants
@robotlolita
Member

robotlolita commented Sep 26, 2016

Folktale has a few functions that are partial (i.e.: not defined for all possible inputs of the types it expects), and when you invoke these functions for an undefined input and exception is thrown. Right now, these functions are not marked in any special way, so code looks like this:

someMaybe.get(); // might throw an exception

In order to make it clear that there might be a problem there, we could use some naming convention. I'm proposing prefixing these functions with unsafe, so you get:

someMaybe.unsafeGet(); // might throw an exception

This would also encourage total functions to be used by default. In the case of get we also provide getOrElse, but it's too tempting to use get because it's more concise. If we change this, that wouldn't be the case anymore:

someMaybe.unsafeGet(); // might throw an exception
someMaybe.getOrElse(null); // always works

Why?

JS does not care much about partiality (many partial functions are provided in the standard library, that either throw an exception or result in nonsensical values), but partiality makes code harder to reason about because the cases where something may fail are not clear from reading just the source code — you also have to read the definitions that you're using, and maybe some external documentation!

Because Folktale aims to provide a better standard library, more aligned with functional programming practices, it would be nice if partiality was thoroughly discouraged. Folktale already discourages partiality in many cases, but there are functions that, historically, have been provided as convenience (such as .get() in Maybe/Either/Validation) that are pretty easy to misuse, in particular because the partial form is much shorter and looks (deceivingly) simpler than the total form.

The impact

While Folktale 2 is a major release, we're trying to keep at least the APIs on the data structures the same as that of Folktale 1, to make it easier for people transitioning from 1 to 2. During this period we'd have the new unsafe<Name> functions as the official API, but would maintain the <name> functions as deprecated aliases, giving people time and directions to migrate from the old API to the new one.

Because this is mostly just a naming change, it doesn't affect any other part of the Folktale library itself.

Affected code

Currently we have the following partial functions:

  • Maybe#get(), Either#get() and Validation#get() (total: #getOrElse(b))
  • mapEntries.unique(o, f) (total: mapEntries.overwrite, new in Folktale 2)
  • Deferred#{resolve,reject,cancel}() (no total equivalent, new in Folktale 2)

Is there anything I've missed or any consideration one'd want to add? Is anyone opposed to this direction?

/cc @boris-marinov @DrBoolean

@robotlolita robotlolita added this to the v2.0.0 milestone Sep 26, 2016

@DrBoolean

This comment has been minimized.

Show comment
Hide comment
@DrBoolean

DrBoolean Sep 26, 2016

I think that sounds great. I tend to use/teach/encourage fold() instead of get() anyways so backward compatibility is simple for those who have adopted this from my workshops.

DrBoolean commented Sep 26, 2016

I think that sounds great. I tend to use/teach/encourage fold() instead of get() anyways so backward compatibility is simple for those who have adopted this from my workshops.

robotlolita added a commit that referenced this issue Jan 7, 2017

robotlolita added a commit that referenced this issue Jan 7, 2017

@robotlolita robotlolita added k:Design and removed k:Discussion labels Jan 11, 2017

robotlolita added a commit that referenced this issue Feb 11, 2017

(result doc) More documentation for Result
Also Renames .errorMap → .mapError

BREAKING CHANGE: The name `.mapError` is a better description for the operation.

Also deprecates .get in favour of .unsafeGet (#42)

robotlolita added a commit that referenced this issue Feb 11, 2017

(result doc) More documentation for Result
Also Renames .errorMap → .mapError

BREAKING CHANGE: The name `.mapError` is a better description for the operation.

Also deprecates .get in favour of .unsafeGet (#42)

robotlolita added a commit that referenced this issue Feb 12, 2017

(result doc) More documentation for Result
Also Renames .errorMap → .mapError

BREAKING CHANGE: The name `.mapError` is a better description for the operation.

Also deprecates .get in favour of .unsafeGet (#42)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment