Add yacht practice exercise #1627
Conversation
|
Hello. Thanks for opening a PR on Exercism 🙂 We ask that all changes to Exercism are discussed on our Community Forum before being opened on GitHub. To enforce this, we automatically close all PRs that are submitted. That doesn't mean your PR is rejected but that we want the initial discussion about it to happen on our forum where a wide range of key contributors across the Exercism ecosystem can weigh in. You can use this link to copy this into a new topic on the forum. If we decide the PR is appropriate, we'll reopen it and continue with it, so please don't delete your local branch. If you're interested in learning more about this auto-responder, please read this blog post. Note: If this PR has been pre-approved, please link back to this PR on the forum thread and a maintainer or staff member will reopen it. |
| @@ -0,0 +1,77 @@ | |||
| class Yacht | |||
There was a problem hiding this comment.
This file should be .meta/example.rb and the path in config.json should be updated too. I'll fix the issue that's causing this.
There was a problem hiding this comment.
FYI, I've fixed the generating code here for future exercises: #1628
| "yacht_test.rb" | ||
| ], | ||
| "example": [ | ||
| ".meta/solutions/yacht.rb" |
There was a problem hiding this comment.
This is the other place it needs fixing.
|
I left a few structural comments. I'll leave @kotp to review the exercise properly. Thanks! |
ccadden
force-pushed
the
add-yacht-practice-exercise
branch
from
53b1ef8 to
6f90e2c
Compare
There was a problem hiding this comment.
Some wording changes with some code changes in the example solution file. The example solution does not need to have "exemplar" status, so any style changes there you are definitely free to ignore. It only provides a "proof of solvability" for the exercise at this point.
Good work so far!
| private | ||
|
|
||
| def count_occurances(num) | ||
| return dice.tally({})[num] || 0 |
There was a problem hiding this comment.
| return dice.tally({})[num] || 0 | |
| dice.tally({})[num] || 0 |
Also, prefer number rather than num for reasons of abbreviations.
| end | ||
|
|
||
| def yacht | ||
| is_yacht? ? 50 : 0 |
There was a problem hiding this comment.
Rubyists usually leave the is_ part of a method to be communicated by the ending ?. yacht? probably should be the preferred name for the query method.
| dice.sum | ||
| end | ||
|
|
||
| def is_yacht? |
There was a problem hiding this comment.
As mentioned prior to this yacht? is probably the preferred name, the question mark repeats the is_ part of the name.
| dice.uniq.length == 1 | ||
| end | ||
|
|
||
| def is_full_house? |
There was a problem hiding this comment.
full_house? is more typical of a method communicating this with the question mark.
| dice.eql?([1, 2, 3, 4, 5]) | ||
| end | ||
|
|
||
| def is_big_straight? |
There was a problem hiding this comment.
Is "big straight" or "large straight" more typical for the game?
| | Little Straight | 30 points | 1-2-3-4-5 | 1 2 3 4 5 scores 30 | | ||
| | Big Straight | 30 points | 2-3-4-5-6 | 2 3 4 5 6 scores 30 | |
There was a problem hiding this comment.
Not sure if little and big are commonly used or "small" and "large" is more commonly used. (Personal experience playing for decades with a small sample of people, rather than a globally applied knowledge.)
There was a problem hiding this comment.
@kotp I'm also more familiar with the "small" and "large" terms from personal experience, but I was just going off what was defined in the canonical data file: https://github.com/exercism/problem-specifications/blob/main/exercises/yacht/canonical-data.json#L9
There was a problem hiding this comment.
That is fine. Noticed it so brought it up here. We can choose to change it for this track specifically or leave it as a potential update from Problem Specifications at some point in the future.
You are doing the work, so I leave it to you.
If you agree with the change, though, feel free to talk about it and maybe someone will tackle the change.
| when 'little straight' then little_straight | ||
| when 'big straight' then big_straight |
There was a problem hiding this comment.
small straight and large straight maybe the changes here.
ccadden
force-pushed
the
add-yacht-practice-exercise
branch
from
6f90e2c to
bac25f6
Compare
|
@kotp thanks for the feedback! I incorporated your suggestions and pushed the updates. |
|
Thank you for the work done. I brought it in regardless of the big/large name. We can always change it later. |
|
Thanks! |
No description provided.