Skip to content

[Bowling] Update implementation and tests #347

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

Merged
merged 1 commit into from
Jul 17, 2017

Conversation

britto
Copy link
Contributor

@britto britto commented Jun 18, 2017

A few changes have been made to this problem since the last update: #416, #423, #444, #536 and more recently #832. My intention here is to reflect all those changes and also make the Elixir exercise as close to canonical as possible.

Non-breaking changes

  • Sync all descriptions with their canonical counterparts.

Possibly breaking changes

  • Add a missing last-frame strike to one of the tests' data.
    • two bonus rolls after a strike in the last frame cannot score more than 10 points
  • Add 4 new tests from the canonical repository.

Breaking changes

  • Update behavior for the scenario where a roll is made after the game is finished.
    • Instead of breaking later on score, we break early on roll (see #536, this commit in particular).
  • Update error messages to also match their canonical counterparts.
    • Pins must have a value from 0 to 10 becomes either Negative roll is invalid or Pin count exceeds pins on the lane.

The reference solution has also been minimally updated to match the aforementioned changes.

Non-breaking changes:

* Sync all descriptions with their canonical counterparts.

Possibly breaking changes:

* Add a missing last-frame strike to one of the tests' data.
* Add 4 new tests from the canonical repository.

Breaking changes:

* Update behavior for the scenario where a roll is made after the game is finished.
* Update error messages to also match their canonical counterparts.
Copy link
Contributor

@devonestes devonestes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given how minimal the breaking changes are (changing some error messages isn't a big deal), I'm cool with this. Thanks for the PR!

Since this does contain some breaking changes, I'm planning on leaving this open for another day or two in case anyone has any input they'd like to offer.

@devonestes devonestes merged commit c6772c9 into exercism:master Jul 17, 2017
@britto britto deleted the bowling-update-implementation branch July 17, 2017 16:49
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