Add exercise yacht #972
Add exercise yacht #972
Conversation
|
I will obviously fix the GCC warning. |
| "blurb": "Score a single throw of dice in the game Yacht.", | ||
| "source": "James Kilfiger, using wikipedia", | ||
| "source_url": "https://en.wikipedia.org/wiki/Yacht_(dice_game)" | ||
| } |
There was a problem hiding this comment.
Nit: could you add a newline at the end of this file?
There was a problem hiding this comment.
Hm, looks like auto-format on save removes it for some reason. It's back now. Sorry for this, I was under the impression that's the way how configlet creates the file and it's how you want it to be.
| { | ||
| TEST_IGNORE(); | ||
| const dice_t dice = { 2, 2, 4, 4, 4 }; | ||
| TEST_ASSERT(score(dice, FULL_HOUSE) == 16); |
There was a problem hiding this comment.
For the test we tend to use the type specific test macros from Unity rather than TEST_ASSERT(). Can you check if there are any suitable for this exercise?
|
|
||
| static void test_not_yacht(void) | ||
| { | ||
| TEST_IGNORE(); |
There was a problem hiding this comment.
In most exercises there's a comment in this first TEST_IGNORE() line that explains that this should be deleted to enable the tests, e.g. in test_difference_of_squares.c
Do you mind adding such comment?
config.json
Outdated
| "uuid": "dd360788-1dff-4a5d-89af-5bbd1199ae86", | ||
| "practices": [], | ||
| "prerequisites": [], | ||
| "difficulty": 2, |
There was a problem hiding this comment.
How about a 4? We have Eliud's eggs rated as a 2 and that's a fair bit less complicated.
| "difficulty": 2, | |
| "difficulty": 4, |
There was a problem hiding this comment.
Whatever you find reasonable. Difficulties are all over the place. spiral-matrix is also a 4 and I would say that's more difficult than this. I updated it to 4 now.
There was a problem hiding this comment.
Agreed, many of these need to be revisited
| } | ||
|
|
||
| typedef struct { | ||
| uint8_t faces[6]; |
There was a problem hiding this comment.
| uint8_t faces[6]; | |
| int faces[6]; |
There's definitely nothing wrong with a fixed sized integer, but it's inconsistent with the typing of the rest of the solution. If we change this to int we should be able to remove stdint.h above.
There was a problem hiding this comment.
If you'd rather use fixed-width types consistently, that's also fine with me.
There was a problem hiding this comment.
Where exactly is this inconsistent? I use uint8_t everywhere except for the final score. Technically, that could also be a uint8_t, but some small change in the specification (let's say yacht is is not 50, but 300 points) would break this.
My main motivation here, though, was efficiency. 6 bytes will easily be passed via a register when doing function calls. 24 bytes could be a different story.
I can change it to int if you prefer. But I'd keep the uint8_t here.
There was a problem hiding this comment.
There are several places in this that use int instead of a fixed-width type. I understand your reasons for making the choice you made, but this is a learning aid, not production code. This optimization will have little meaning to someone scoring a dice game to learn the fundamentals of C. My thought is that seeing a mix of fixed-width and non-fixed-width types will be more surprising to someone solving the problem than a missed optimization.
| #include "yacht.h" | ||
|
|
||
| #include <assert.h> | ||
| #include <stdbool.h> |
There was a problem hiding this comment.
| #include <stdbool.h> |
I think this is unused
There was a problem hiding this comment.
A leftover from when has_count returned a bool. Removed that now.
| #ifndef YACHT_H | ||
| #define YACHT_H | ||
|
|
||
| #include <stdint.h> |
There was a problem hiding this comment.
I think this include can probably be removed now?
Then I think this PR is good to go!
There was a problem hiding this comment.
Ah, right! Hadn't had a chance to look at the diff myself yet! This one is gone now as well.
|
LGTM, well done @ahans ! |
|
Thanks! |
No description provided.