[New practice exercise] Gilded Rose #1586
Conversation
|
Hello. Thanks for opening a PR on Exercism. We are currently in a phase of our journey where we have paused community contributions to allow us to take a breather and redesign our community model. You can learn more in this blog post. As such, all issues and PRs in this repository are being automatically closed. That doesn't mean we're not interested in your ideas, or that if you're stuck on something we don't want to help. The best place to discuss things is with our community on the Exercism Community Forum. You can use this link to copy this into a new topic there. 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. |
|
@iHiD Tagging you as you requested in the forum thread. |
|
|
||
| - **_Conjured_** items degrade in quality twice as fast as they would otherwise. However, special rules apply for conjured items that are not normal items: | ||
| - **_Conjured Aged Brie_** and **_Conjured Sulfuras_** behave as their non-conjured counterparts, except that once their sell-by date has arrived, their quality degrades at the rate of a conjured normal item. (Which means, of course, a conjured Sulfuras has a sell-by date.) | ||
| - **_Conjured backstage passes_** increase in quality by 1 less than if they were not conjured. |
There was a problem hiding this comment.
This seems a bit confusing to me, could you reword this?
There was a problem hiding this comment.
Done in 23d834b. If you think the new feature requirements are just too complex, let me know and I can simplify them.
The new feature in the original Gilded Rose is a lot simpler, which makes it easier to take a shortcut and avoid refactoring. (See my thoughts on this in design.md.) But maybe I'm worrying about that too much.
There was a problem hiding this comment.
I decided to simplify the new feature a bit in 5d83d2c. Now it's easier to understand (I hope) while still giving students the extra motivation to refactor first.
There was a problem hiding this comment.
@vaeng You happy with the current wording?
|
|
||
| ## General | ||
|
|
||
| - If you're not sure how to start the refactor, watch the talk ["All the Little Things"][all-the-little-things] by Sandi Metz, and pause it as soon as you find inspiration. Try not to watch the whole talk until after you've solved the exercise! Then you can watch the rest of the talk and improve upon your first iteration. |
| def test_normal_item_on_sell_date | ||
| update_with sell_in: 0, quality: 10, name: "some item", quality_change: -2 | ||
| end | ||
|
|
||
| def test_normal_item_after_sell_date | ||
| update_with sell_in: -1, quality: 10, name: "some item", quality_change: -2 | ||
| end | ||
|
|
||
| def test_normal_item_of_zero_quality | ||
| update_with sell_in: 1, quality: 0, name: "some item", quality_change: 0 | ||
| end | ||
|
|
||
| def test_normal_item_near_zero_quality | ||
| update_with sell_in: 0, quality: 1, name: "some item", quality_change: -1 | ||
| end | ||
|
|
||
| def test_brie_before_sell_date | ||
| update_with sell_in: 1, quality: 0, name: "Aged Brie", quality_change: 1 | ||
| end | ||
|
|
||
| def test_brie_on_sell_date | ||
| update_with sell_in: 0, quality: 0, name: "Aged Brie", quality_change: 2 | ||
| end | ||
|
|
||
| def test_brie_after_sell_date | ||
| update_with sell_in: -1, quality: 0, name: "Aged Brie", quality_change: 2 | ||
| end | ||
|
|
||
| def test_brie_of_max_quality | ||
| update_with sell_in: 1, quality: 50, name: "Aged Brie", quality_change: 0 | ||
| end | ||
|
|
||
| def test_brie_near_max_quality | ||
| update_with sell_in: 0, quality: 49, name: "Aged Brie", quality_change: 1 | ||
| end | ||
|
|
||
| def test_sulfuras_before_sell_date | ||
| update_with sell_in: 1, quality: 80, name: "Sulfuras, Hand of Ragnaros", | ||
| sell_in_change: 0, quality_change: 0 | ||
| end | ||
|
|
||
| def test_sulfuras_on_sell_date | ||
| update_with sell_in: 0, quality: 80, name: "Sulfuras, Hand of Ragnaros", | ||
| sell_in_change: 0, quality_change: 0 | ||
| end | ||
|
|
||
| def test_sulfuras_after_sell_date | ||
| update_with sell_in: -1, quality: 80, name: "Sulfuras, Hand of Ragnaros", | ||
| sell_in_change: 0, quality_change: 0 | ||
| end | ||
|
|
||
| def test_backstage_pass_far_from_sell_date | ||
| update_with sell_in: 11, quality: 10, name: "Backstage passes to a TAFKAL80ETC concert", | ||
| quality_change: 1 | ||
| end | ||
|
|
||
| def test_backstage_pass_medium_from_sell_date_upper_bound | ||
| update_with sell_in: 10, quality: 0, name: "Backstage passes to a TAFKAL80ETC concert", | ||
| quality_change: 2 | ||
| end | ||
|
|
||
| def test_backstage_pass_medium_from_sell_date_lower_bound | ||
| update_with sell_in: 6, quality: 0, name: "Backstage passes to a TAFKAL80ETC concert", | ||
| quality_change: 2 | ||
| end | ||
|
|
||
| def test_backstage_pass_close_to_sell_date_upper_bound | ||
| update_with sell_in: 5, quality: 0, name: "Backstage passes to a TAFKAL80ETC concert", | ||
| quality_change: 3 | ||
| end | ||
|
|
||
| def test_backstage_pass_close_to_sell_date_lower_bound | ||
| update_with sell_in: 1, quality: 0, name: "Backstage passes to a TAFKAL80ETC concert", | ||
| quality_change: 3 | ||
| end | ||
|
|
||
| def test_backstage_pass_on_sell_date | ||
| update_with sell_in: 0, quality: 10, name: "Backstage passes to a TAFKAL80ETC concert", | ||
| quality_change: -10 | ||
| end | ||
|
|
||
| def test_backstage_pass_after_sell_date | ||
| update_with sell_in: -1, quality: 10, name: "Backstage passes to a TAFKAL80ETC concert", | ||
| quality_change: -10 | ||
| end | ||
|
|
||
| def test_backstage_pass_of_max_quality | ||
| update_with sell_in: 11, quality: 50, name: "Backstage passes to a TAFKAL80ETC concert", | ||
| quality_change: 0 | ||
| end | ||
|
|
||
| def test_backstage_pass_near_max_quality | ||
| update_with sell_in: 1, quality: 49, name: "Backstage passes to a TAFKAL80ETC concert", | ||
| quality_change: 1 | ||
| end |
There was a problem hiding this comment.
These tests are all missing the skip part
There was a problem hiding this comment.
There was a problem hiding this comment.
Well, maybe they shouldn't. But then they should all be unskipped
|
Note that there's various CI failures here. |
@iHiD AFAICT the CI failures are from linter warnings on the starting code, which are to be expected. (With one exception; see my "Note on the linter warnings" in the PR description.) Is there a way to ignore those warnings? |
|
You can add exceptions to https://github.com/exercism/ruby/blob/main/.rubocop.yml |
https://forum.exercism.org/t/add-the-gilded-rose-kata/7758
This version is a bit different from the classic Gilded Rose Kata. See 1094e53 for the changes of substance (EDIT: simplified, see discussion below), and 6d03473 for the rationale. Feedback on this adjustment is welcome!
I've also edited the instructions from the original for clarity, conciseness, and emphasis on the task of refactoring.
### Note on the linter warningsAll of the warnings are about the starting code (which is supposed to be horrible), except for one, an "Ambiguous negative number operator" warning on a line which (IMO) is unambiguous.EDIT: Added a RuboCop exclusion for the starting code, and adjusted the code re: the other warning.