emilybache / gildedrose-refactoring-kata

Starting code for the GildedRose Refactoring Kata in many programming languages.

Support this and all my katas via Patreon

Gilded Rose Refactoring Kata

You can find out more about this exercise in my YouTube video Why Developers LOVE The Gilded Rose Kata. I also have a video of a worked solution in Java - Gilded Rose Kata, Hands-on

I use this kata as part of my work as a technical coach. I wrote a lot about the coaching method I use in this book Technical Agile Coaching with the Samman method. A while back I wrote this article "Writing Good Tests for the Gilded Rose Kata" about how you could use this kata in a coding dojo.

How to use this Kata

The simplest way is to just clone the code and start hacking away improving the design. You'll want to look at the "Gilded Rose Requirements" which explains what the code is for. I strongly advise you that you'll also need some tests if you want to make sure you don't break the code while you refactor.

You could write some unit tests yourself, using the requirements to identify suitable test cases. I've provided a failing unit test in a popular test framework as a starting point for most languages.

Alternatively, use the Approval tests provided in this repository. (Read more about that in the section "Text-based Approval Testing").

The idea of the exercise is to do some deliberate practice, and improve your skills at designing test cases and refactoring. The idea is not to re-write the code from scratch, but rather to practice taking small steps, running the tests often, and incrementally improving the design.

Please don't send me a pull request with your solution. It can be a bit confusing since GitHub encourages you to do so! Please only send me pull requests if you have a correction or improvement to the starting position. You don't want to spoil the fun of doing the exercise for other people!

Gilded Rose Requirements in other languages

• English
• Español
• Français
• Italiano
• 日本語
• Português
• Русский
• ไทย
• 中文
• 한국어
• German
• Euskara

Text-Based Approval Testing

Most language versions of this code have a TextTest fixture for Approval testing. For information about this, see the TextTests README

History of the exercise

This Kata was originally created by Terry Hughes (http://twitter.com/TerryHughes). It is already on GitHub here. See also Bobby Johnson's description of the kata.

I translated the original C# into a few other languages, (with a little help from my friends!), and slightly changed the starting position. This means I've actually done a small amount of refactoring already compared with the original form of the kata, and made it easier to get going with writing tests by giving you one failing unit test to start with. I also added test fixtures for Text-Based approval testing with TextTest (see the TextTests)

As Bobby Johnson points out in his article "Why Most Solutions to Gilded Rose Miss The Bigger Picture", it'll actually give you better practice at handling a legacy code situation if you do this Kata in the original C#. However, I think this kata is also really useful for practicing writing good tests using different frameworks and approaches, and the small changes I've made help with that. I think it's also interesting to compare what the refactored code and tests look like in different programming languages.

Translating this code

More translations are most welcome! I'm very open for pull requests that translate the starting position into additional languages.

Please note a translation should ideally include:

• a translation of the production code for 'update_quality' and Item
• one failing unit test complaining that "fixme" != "foo"
• a TextTest fixture, ie a command-line program that runs update_quality on the sample data for the number of days specified.

Please don't write too much code in the starting position or add too many unit tests. The idea with the one failing unit test is to tempt people to work out how to fix it, discover it wasn't that hard, and now they understand what this test is doing they realize they can improve it.

If your programming language doesn't have an easy way to add a command-line interface, then the TextTest fixture is probably not necessary.

gildedrose-refactoring-kata's Issues

Swift sample items have copy/paste errors

The swift version of the test data in swift/main.swift has 2 copy/paste errors. PR coming up to fix.

TextTestHarness vs Unit treatment of items

I did the the Java - Spock version of the Kata. The starter unit test runs against app.items, but the test harness assumes that the original inputs will be changed.

Based on the initial unit test I built my solution around access to the property, not on side effects to the original inputs. So when I ran the TextTestHarness (TextDemoApp would be a better name) things broke. Just had to change the harness to app.items and things worked as expected.

Is this an issue? An intended part of the kata? I'm willing to do a PR to change it in the Java versions if you think it's an issue.

Duplication

GildedRose-Refactoring-Kata/ruby/gilded_rose.rb

Line 7 in bdcea60

def update_quality()

Rider doesn't detect unit tests in csharpcore exercise

Rider doesn't detect the NUnit unit tests in the csharpcore project, possibly due to the project setup. Because the unit test project is configured as a console application, I assume that the unit tests aren't being recognized. It's not commonplace to have your unit test project be a console application.

My suggested fix is to have three projects:

• main class library, GildedRose
• unit test project, GildedRoseUnitTests
• approval testing project, GildedRoseApprovalTests

Further instructions for the approval testing also wouldn't go amiss as it's not clear how they should be used.

texttest_fixture.py Doesn't follow requirements?

Hey, sorry I'm new to git, so I'm not sure what the best practices are but I found a possible issue.

In your texttest_fixture.py, you've got the quality set to 49 for the backstage passes:

Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=10, quality=49),
Item(name="Backstage passes to a TAFKAL80ETC concert", sell_in=5, quality=49),

In your GildedRoseRequirements.txt, you've written:
The Quality of an item is never more than 50

The output of texttest_fixture.py, indicates that the quality of the backstage passes become 51 and 52, should the output not be 50? Since that's one of the constraints that's been applied?

dotnet core test is not running on Windows plateform

Hello,

There's an issue in the dotnet core test on Windows only (I guess), for some reasons the console stdout is changing the carriage return sequence, and the test can't be validated.

[cpp] tests not compiling for Catch2

Using

[cmake] -- The C compiler identification is GNU 10.4.0
[cmake] -- The CXX compiler identification is GNU 10.4.0

from ubuntu 22.04 causes compilation issues:

full output:

root@fa43466bd319:/workspaces/GildedRose-Refactoring-Kata/build# cd ../cpp
root@fa43466bd319:/workspaces/GildedRose-Refactoring-Kata/cpp# mkdir build
root@fa43466bd319:/workspaces/GildedRose-Refactoring-Kata/cpp# cd build
root@fa43466bd319:/workspaces/GildedRose-Refactoring-Kata/cpp/build# cmake ..
-- The C compiler identification is GNU 10.4.0
-- The CXX compiler identification is GNU 10.4.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
cmake --build CMake Deprecation Warning at build/_deps/googletest-src/CMakeLists.txt:1 (cmake_minimum_required):
  Compatibility with CMake < 2.8.12 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.


.CMake Deprecation Warning at build/_deps/googletest-src/googlemock/CMakeLists.txt:42 (cmake_minimum_required):
  Compatibility with CMake < 2.8.12 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.


CMake Deprecation Warning at build/_deps/googletest-src/googletest/CMakeLists.txt:49 (cmake_minimum_required):
  Compatibility with CMake < 2.8.12 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.


-- Found PythonInterp: /usr/bin/python (found version "3.10.6") 
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE  
-- Configuring done
-- Generating done
-- Build files have been written to: /workspaces/GildedRose-Refactoring-Kata/cpp/build
root@fa43466bd319:/workspaces/GildedRose-Refactoring-Kata/cpp/build# cmake --build .
[18/21] Building CXX object test/cpp_catch2_unittest/CMakeFiles/GildedRoseCatch2UnitTests.dir/GildedRoseCatch2UnitTests.cc.o
FAILED: test/cpp_catch2_unittest/CMakeFiles/GildedRoseCatch2UnitTests.dir/GildedRoseCatch2UnitTests.cc.o 
/usr/bin/c++  -I/workspaces/GildedRose-Refactoring-Kata/cpp/lib -I/workspaces/GildedRose-Refactoring-Kata/cpp/src -std=gnu++11 -MD -MT test/cpp_catch2_unittest/CMakeFiles/GildedRoseCatch2UnitTests.dir/GildedRoseCatch2UnitTests.cc.o -MF test/cpp_catch2_unittest/CMakeFiles/GildedRoseCatch2UnitTests.dir/GildedRoseCatch2UnitTests.cc.o.d -o test/cpp_catch2_unittest/CMakeFiles/GildedRoseCatch2UnitTests.dir/GildedRoseCatch2UnitTests.cc.o -c /workspaces/GildedRose-Refactoring-Kata/cpp/test/cpp_catch2_unittest/GildedRoseCatch2UnitTests.cc
In file included from /usr/include/signal.h:328,
                 from /workspaces/GildedRose-Refactoring-Kata/cpp/lib/Catch.hpp:7937,
                 from /workspaces/GildedRose-Refactoring-Kata/cpp/test/cpp_catch2_unittest/GildedRoseCatch2UnitTests.cc:2:
/workspaces/GildedRose-Refactoring-Kata/cpp/lib/Catch.hpp:10717:58: error: call to non-‘constexpr’ function ‘long int sysconf(int)’
10717 |     static constexpr std::size_t sigStackSize = 32768 >= MINSIGSTKSZ ? 32768 : MINSIGSTKSZ;
      |                                                          ^~~~~~~~~~~
In file included from /usr/include/x86_64-linux-gnu/bits/sigstksz.h:24,
                 from /usr/include/signal.h:328,
                 from /workspaces/GildedRose-Refactoring-Kata/cpp/lib/Catch.hpp:7937,
                 from /workspaces/GildedRose-Refactoring-Kata/cpp/test/cpp_catch2_unittest/GildedRoseCatch2UnitTests.cc:2:
/usr/include/unistd.h:640:17: note: ‘long int sysconf(int)’ declared here
  640 | extern long int sysconf (int __name) __THROW;
      |                 ^~~~~~~
In file included from /workspaces/GildedRose-Refactoring-Kata/cpp/test/cpp_catch2_unittest/GildedRoseCatch2UnitTests.cc:2:
/workspaces/GildedRose-Refactoring-Kata/cpp/lib/Catch.hpp:10776:45: error: size of array ‘altStackMem’ is not an integral constant-expression
10776 |     char FatalConditionHandler::altStackMem[sigStackSize] = {};
      |                                             ^~~~~~~~~~~~
[19/21] Building CXX object test/cpp_catch2_approvaltest/CMakeFiles/GildedRoseCatch2ApprovalTests.dir/GildedRoseCatch2ApprovalTests.cc.o
FAILED: test/cpp_catch2_approvaltest/CMakeFiles/GildedRoseCatch2ApprovalTests.dir/GildedRoseCatch2ApprovalTests.cc.o 
/usr/bin/c++  -I/workspaces/GildedRose-Refactoring-Kata/cpp/lib -I/workspaces/GildedRose-Refactoring-Kata/cpp/src -std=gnu++11 -MD -MT test/cpp_catch2_approvaltest/CMakeFiles/GildedRoseCatch2ApprovalTests.dir/GildedRoseCatch2ApprovalTests.cc.o -MF test/cpp_catch2_approvaltest/CMakeFiles/GildedRoseCatch2ApprovalTests.dir/GildedRoseCatch2ApprovalTests.cc.o.d -o test/cpp_catch2_approvaltest/CMakeFiles/GildedRoseCatch2ApprovalTests.dir/GildedRoseCatch2ApprovalTests.cc.o -c /workspaces/GildedRose-Refactoring-Kata/cpp/test/cpp_catch2_approvaltest/GildedRoseCatch2ApprovalTests.cc
In file included from /usr/include/signal.h:328,
                 from /workspaces/GildedRose-Refactoring-Kata/cpp/lib/Catch.hpp:7937,
                 from /workspaces/GildedRose-Refactoring-Kata/cpp/lib/ApprovalTests.v.6.0.0.hpp:2422,
                 from /workspaces/GildedRose-Refactoring-Kata/cpp/lib/ApprovalTests.hpp:3,
                 from /workspaces/GildedRose-Refactoring-Kata/cpp/test/cpp_catch2_approvaltest/GildedRoseCatch2ApprovalTests.cc:2:
/workspaces/GildedRose-Refactoring-Kata/cpp/lib/Catch.hpp:10717:58: error: call to non-‘constexpr’ function ‘long int sysconf(int)’
10717 |     static constexpr std::size_t sigStackSize = 32768 >= MINSIGSTKSZ ? 32768 : MINSIGSTKSZ;
      |                                                          ^~~~~~~~~~~
In file included from /workspaces/GildedRose-Refactoring-Kata/cpp/lib/ApprovalTests.v.6.0.0.hpp:271,
                 from /workspaces/GildedRose-Refactoring-Kata/cpp/lib/ApprovalTests.hpp:3,
                 from /workspaces/GildedRose-Refactoring-Kata/cpp/test/cpp_catch2_approvaltest/GildedRoseCatch2ApprovalTests.cc:2:
/usr/include/unistd.h:640:17: note: ‘long int sysconf(int)’ declared here
  640 | extern long int sysconf (int __name) __THROW;
      |                 ^~~~~~~
In file included from /workspaces/GildedRose-Refactoring-Kata/cpp/lib/ApprovalTests.v.6.0.0.hpp:2422,
                 from /workspaces/GildedRose-Refactoring-Kata/cpp/lib/ApprovalTests.hpp:3,
                 from /workspaces/GildedRose-Refactoring-Kata/cpp/test/cpp_catch2_approvaltest/GildedRoseCatch2ApprovalTests.cc:2:
/workspaces/GildedRose-Refactoring-Kata/cpp/lib/Catch.hpp:10776:45: error: size of array ‘altStackMem’ is not an integral constant-expression
10776 |     char FatalConditionHandler::altStackMem[sigStackSize] = {};
      |                                             ^~~~~~~~~~~~
ninja: build stopped: subcommand failed.
root@fa43466bd319:/workspaces/GildedRose-Refactoring-Kata/cpp/build# 

cpp cpptexttest shows unidiomatic C++

I would think, that the text output could make use of iostreams or fmt instead of using C's printf. but may be that is part of the refactoring exercise?

Create `with_tests` branch

I've noticed on the Supermarket Receipt Refactoring Kata a with_tests branch has been optionally provided. I was thinking of creating the same for this Kata (PHP version), which, as that will allow jumping directly into the refactoring part of this exercise too.

I wanted to get your opinion, as this is the most stared Kata, and most versions, adding a new branch may create more work for you.

I was also going to update the PHP version to the same tooling and similar README as the others I've recently done, which would keep the install method and tooling consistent across Katas.

Conjured items are under specified

The spec here is not well enough defined for "Conjured Items".

• Does Brie "appreciate" twice as fast when conjured?
• Do backstage passes accrue additional value twice as fast near the time of the concert?

I suppose that finding these learning and collaboration exercises might be the point of this kata. Just wanted to take the time to say how much i appreciated and enjoyed the experience. Thank you for maintaining this. ❤️

Error on stdout.gr for conjured items expected quality

I believe there is an error on the file stdout.gr for the texttests for the expected values on conjured items (the new feature):

-------- day 0 --------
Conjured Mana Cake, 3, 6
-------- day 1 --------
Conjured Mana Cake, 2, 5
-------- day 2 --------
Conjured Mana Cake, 1, 4
-------- day 3 --------
Conjured Mana Cake, 0, 3
-------- day 4 --------
Conjured Mana Cake, -1, 1
-------- day 5 --------
Conjured Mana Cake, -2, 0
-------- day ... --------

The requirement is "Conjured" items degrade in Quality twice as fast as normal items, so while a normal item quality degrades by 1 each day, the conjured items should degrade by 2 and while normal items quality degrades by 2 once the sell by date has passed, the conjured items should degrade by 4.

Given that, a test suit for conjured items could be:

-------- day 0 --------
Conjured Mana Cake, 3, 10
-------- day 1 --------
Conjured Mana Cake, 2, 8
-------- day 2 --------
Conjured Mana Cake, 1, 6
-------- day 3 --------
Conjured Mana Cake, 0, 4
-------- day 4 --------
Conjured Mana Cake, -1, 0
-------- day 5 --------
Conjured Mana Cake, -2, 0
-------- day ... --------

Links at the bottom of the page don't work

links like Ruby gives us Rails routing error

[elixir] Is this one correctly implemented?

I wanted to tackle this kata as I heard so much about it and was happy that there is an elixir version available in this repositoriy.

So I thought, "lets start with that failing test!" – Nope… No failing tests, only a single one which is empty, it does test even less than an asserting true…

So move on… Just write tests…

First one was that an items quality is reduced by 1, check! worked well…
Second one was to check if sell_in of an item is decremented, and guess… It is not. So I took a look into the corresponding source and found out, that there is only reading access to that field, but no writing access. So I took a look at the ruby sources, which in fact do update that field/attribute, so missing it seems not to be on purpose.

Maybe there are other flaws I haven't found so far…

[PHP] Item class should not be final

When refactoring the PHP version from PHP 5 to PHP 7, the Item class was made final, but no interface was introduced.

GildedRose-Refactoring-Kata/php/src/Item.php

Line 7 in d462c36

final class Item implements \Stringable

Given that:

• the Item class cannot be extended now
• the Item is not an interface
• the GildedRose class directly depends on the concrete Item class instead of an interface

... this all makes it technically wrong to use any proper OOP techniques for refactoring this in such a way that items can include any properties about themselves, forcing you to hardcode all behavior into the GildedRose or one of its parts in some way. Using final without any interfaces in-between generally makes for the type of bad couplig you cannot easily rectify in PHP.

Note: you can still do these types of operations, as the type is not technically enforced, but this will break static analysis and be conceptually wrong.

I would suggest one of two approaches to allow for better refactoring opportunities in the PHP version:

1. Remove the final keyword from the Item class.
   This would allow something like LegendaryItem extends Item to still be accepted in the shop class (which requires items to be of type Item as per its docblock).

2. Turn Item into an interface and provide something like a BaseItem implementation.
   This would allow the shop to type against an interface instead, which opens up refactoring methods like using decorators: new ConjuredItem( new Item( 'Mana Cake', 1, 4 ) ).

Link to article broken

First of all: excellent idea! Although this kata has become a classic among the software-craftsmanship circles by now, I wanted to congratulate you on the idea and the initiative nevertheless. Probably we will be doing this kata at our company soon.

What I wanted to point out is that the link to the article "Why Most Solutions to Gilded Rose Miss The Bigger Picture", mentioned in the README, seems to be broken (the article does not seem to be available anymore).

I could only access it by consulting Google's cache:

http://webcache.googleusercontent.com/search?q=cache:ot2ak8m5LUkJ:iamnotmyself.com/2012/12/07/why-most-solutions-to-gilded-rose-miss-the-bigger-picture/+&cd=1&hl=en&ct=clnk&gl=at

If the article is "gone forever", maybe you could compensate it by mentioning the message of the article: that getting a piece of legacy code in a language different than the one you are familiar with is part of the challenge (of dealing with legacy systems) itself.

Update for Swift 3 support

Apple have removed the Process enum in Swift 3, and replaced it with CommandLine. I'll send a PR for the fix to the main.swift file.

octave / matlab ?

was wondering if it could be interesting to add a version of the kata for octave / matlab?

Add Gilded Rose kata in JavaScript with Jest

We use the Gilded Rose kata to teach developers how to write tests and refactor the code. We use JavaScript with Jest. Right now there are katas in JavaScript with Mocha and Jasmine, but none for Jest.

Java version seems to update tickets incorrectly

While testing my own code against the original I believe I've noticed an incorrect edge case.
When tickets have sellIn==10, it is supposed to increase quality by 2 but it does not, it works from 9 and below just fine, same for 5 and below.

texttest quality values for Conjured Mana Cake incorrect

Based on the requirements

• "Conjured" items degrade in Quality twice as fast as normal items

TextTests "ThirtyDays" Test Contains 31 Days

Hello,

It might by knit-picking but the number of days contained in texttests/ThirtyDays/stdout.gr is actually 31 although the title suggests 30 entries.

Regards,
Sven Heyll

Aged Brie getting 2 points per time step ?

Hi,

I'm wondering if this is normal or not, the code provided does +=2 on the quality of "aged brie" at each step, which the requirements don't seem to express.

It's not obvious in the source code, but after refactoring in my solution (without touching semantics) it was glaringly obvious.

There are two updates to quality :

• https://github.com/emilybache/GildedRose-Refactoring-Kata/blob/master/csharp/GildedRose.cs#L31
• https://github.com/emilybache/GildedRose-Refactoring-Kata/blob/master/csharp/GildedRose.cs#L82

I thought the kata was about the new feature, but that existing code was supposedly semantically correct.

thanks for the work on these kata.

texttest conjured items

The instructions state that conjured items degrade twice as fast as normal items, but the texttest stdout.gr indicates that they degrade at the normal item rate. Therefore, I believe there may be an issue with the conjured items.

csharpcore Acceptance Tests stay blue (not executed)

Hi,

I am maybe super new in C#, and NUnit and Acceptance Testing... so, pardon my newbeness.
After running the .sln csharpcore project, I don't obtain the .txt of the approval tests.
And my approval tests stays blue (meaning not executed).
But the unit tests pass.

Is the project badly configured, or is there a pre-requisit I miss?

Link to Better Code Hub / BCH compliance badge broken in README.md

When opening https://bettercodehub.com/ it says that Better Code Hub has retired. The BCH compliance badge isn't working.