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

Error tuples #24

Open
wants to merge 33 commits into
base: master
Choose a base branch
from
Open

Error tuples #24

wants to merge 33 commits into from

Conversation

GeoffreyPS
Copy link
Collaborator

This PR corresponds to issue #23 (Error Conventions)

If merged, this commit will:

  • change returns of most functions from result to either {:ok, result} or {:error, reason}.
  • functions that are composed of other Statisaur functions have slightly different internal implementations, typically leveraging the with macro, but may use try/rescue blocks in rare instances.
  • add bang versions of most functions (e.g. adds median/1 and median!/1).
  • add new test cases for error testing; alter test cases to correspond with new return values.

These changes extend to the original Statisaur module and Statisaur.Combinatorics module, as they are the only modules on the master branch.

Please let me know if there are other actions I need to take before this can be merged (bumping version number, fixing anything, etc). Thanks for taking the time to review!

@GeoffreyPS
Copy link
Collaborator Author

I just remembered that this will also require use of Elixir version 1.2 or higher with use of the with macro in the implementation of some functions. After discussion, I can go ahead and bump the required Elixir version within mix.

…ng variants.

To do:
update falling_factorial and rising_factorial to include error tuples; update n_choose_k to use new falling_factorial function.
to do:
Extend test cases for other functions in the module.
…ng variants

to do:
Include cases where lists contain invalid information (like lists of non-number terms).
Extend {:ok, reponse} return values for the remaining functions in the module.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants