Refactor GildedRose: extract methods, eliminate nesting, add test coverage

[Copilot]

Gilded Rose Refactoring - Clean Code Implementation ✅

Phase 1: Understanding Business Rules ✅

• Review README and understand business rules
• Identify all item types: Normal, Aged Brie, Backstage passes, Sulfuras
• Document quality bounds (0-50, except Sulfuras)
• Understand sellIn behavior

Phase 2: Create Comprehensive Tests ✅

• Create tests directory structure
• Test Normal items (quality degrades by 1, then by 2 after sell date, never negative)
• Test Aged Brie (quality increases, never exceeds 50)
• Test Backstage passes (increases by 1/2/3, drops to 0 after concert)
• Test Sulfuras (never changes)
• Test edge cases (quality bounds, sellIn=0, negative quality prevention)
• Run tests to ensure current code passes (21 tests, 48 assertions - ALL PASS ✅)

Phase 3: Refactor Code Following Clean Code Principles ✅

• Extract magic strings to constants (AGED_BRIE, BACKSTAGE_PASSES, SULFURAS)
• Extract magic numbers to constants (MAX_QUALITY, MIN_QUALITY, thresholds)
• Improve variable naming and method names
• Simplify nested conditionals with guard clauses (isSulfuras early return)
• Extract methods for each item type behavior
• Remove code duplication (increaseQuality, decreaseQuality methods)
• Convert if-elseif-else to guard clauses in updateQualityAfterSellIn
• Run tests after each change (ALL TESTS PASS ✅)
• Verify example.php output remains correct

Phase 4: Final Validation ✅

• Run full test suite (21 tests, 48 assertions - ALL PASS ✅)
• Verify code readability improvements (clear method names, reduced complexity)
• Ensure no behavior changes (example.php output identical)
• Code review (completed and addressed feedback)
• Security scan (no issues found)

🎯 Latest Changes

• Refactored updateQualityAfterSellIn to use guard clauses instead of if-elseif-else
• Improved code readability with early returns
• All tests continue to pass

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Refactorizar kata siguiendo clean code</issue_title>
<issue_description># 🎯 Objetivo de la issue

Refactorizar el código de Gilded Rose para mejorar su legibilidad, mantenibilidad y extensibilidad, sin cambiar el comportamiento observable del sistema.

El refactor debe estar guiado por tests, asegurando que las reglas de negocio actuales estén correctamente cubiertas y que cualquier cambio estructural mantenga el comportamiento intacto.

📌 Contexto

El código actual de Gilded Rose:

• Contiene lógica compleja y condicionales anidados.
• Es difícil de extender para nuevos tipos de items.
• No expresa claramente las reglas de negocio.

Antes de refactorizar, es imprescindible proteger el comportamiento actual con tests significativos, evitando tests redundantes o de bajo valor.

🧪 Paso 1: Entender las reglas de negocio

Antes de escribir tests o refactorizar:

• Leer cuidadosamente la descripción del kata de Gilded Rose.
• Identificar y listar explícitamente las reglas de negocio, por ejemplo:
  • Items normales degradan su quality en 1 por día.
  • Una vez pasada la fecha de venta (sellIn < 0), la degradación se duplica.
  • Aged Brie incrementa su quality.
  • Backstage passes incrementan su quality según el sellIn y caen a 0 después del concierto.
  • Sulfuras no cambia ni sellIn ni quality.
  • quality nunca es negativa ni mayor a 50 (excepto Sulfuras).

Estas reglas deben reflejarse en los tests.

🧪 Paso 2: Crear tests antes de refactorizar

Principios para los tests

❌ No crear tests innecesarios o duplicados.

✅ Crear tests que:

• Cubran todas las reglas de negocio.
• Cubran casos límite (bordes).
• Sirvan como red de seguridad para el refactor.

Qué tipos de tests escribir

• Tests por tipo de item (no por método).
• Tests enfocados en comportamiento, no en implementación.
• Tests claros y expresivos (el nombre del test debe explicar la regla).

Ejemplos de casos a cubrir

Item normal:

• Degrada quality en 1 antes del sell date.
• Degrada quality en 2 después del sell date.
• Nunca baja de 0.

Aged Brie:

• Incrementa quality cada día.
• Nunca supera 50.
• Backstage passes:
• Incrementa en +1 cuando sellIn > 10.
• Incrementa en +2 cuando sellIn <= 10.
• Incrementa en +3 cuando sellIn <= 5.
• Quality pasa a 0 cuando sellIn < 0.

Sulfuras:

• No cambia ni sellIn ni quality.

📌 Importante:
Antes de continuar, todos los tests deben pasar en el código actual.

✅ Paso 3: Verificar cobertura y confianza

Ejecutar los tests y asegurarse de que:

• Todas las reglas de negocio están cubiertas.
• Los casos límite están testeados.
• Revisar cobertura de código:
• La cobertura es un indicador, no el objetivo.
• Priorizar cobertura de reglas, no líneas.
• Confirmar que los tests fallarían si se rompe alguna regla de negocio.

🔧 Paso 4: Refactorizar paso a paso

Refactorizar en pequeños pasos, ejecutando los tests constantemente.

Posibles pasos (no obligatorios, pero recomendados):

• Buscar un mejor naming para las variables
• Simplificar condicionales complejos y usar clausulas de guarda.
• Extraer métodos con nombres expresivos.
• Eliminar código duplicado.
• Eliminar magic numbers/strings
• Aplicar polimorfismo o estrategia por tipo de item.
• Introducir clases específicas por item si aporta claridad.

Mantener la lógica de negocio explícita y legible.

⚠️ En ningún momento deben romperse los tests existentes y lo suyo sería que por cada cambio que vayas haciendo corras los test para asegurar que no se ha roto nada

🔁 Paso 5: Validación final

Ejecutar toda la suite de tests.

Verificar que:

• No se han añadido tests redundantes.
• El código es más fácil de entender que antes.
• Añadir un nuevo tipo de item sería sencillo.
• Revisar que no haya cambios en el comportamiento funcional.

📦 Resultado esperado

Código refactorizado, más limpio y mantenible con tests que documentan las reglas de negocio.
Comportamiento intacto.
Base preparada para futuras extensiones.</issue_description>

<agent_instructions>Quiero que actues como un software engineer senior, que te bases en los principios y buenas practicas y que refactorices el código actual buscando mejorar su legibiliadad, haciendolo escalable y siguiendo clean code</agent_instructions>

Comments on the Issue (you are @copilot in this section)

• Fixes Refactorizar kata siguiendo clean code #1

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[amlndz]

@copilot revisa esto y cambialo a clausulas de guarda

[Copilot]

Done! Refactored to use guard clauses with early returns for better readability. (4f41134)

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• https://api.github.com/repos/sebastianbergmann/environment/zipball/8074dbcd93529b357029f5cc5058fd3e43666984
  • Triggering command: /usr/bin/php8.3 /usr/bin/php8.3 -n -c /tmp/PLAbBE /usr/bin/composer install --no-interaction --prefer-dist (http block)
• https://api.github.com/repos/sebastianbergmann/exporter/zipball/0735b90f4da94969541dac1da743446e276defa6
  • Triggering command: /usr/bin/php8.3 /usr/bin/php8.3 -n -c /tmp/PLAbBE /usr/bin/composer install --no-interaction --prefer-dist (http block)
• https://api.github.com/repos/sebastianbergmann/global-state/zipball/987bafff24ecc4c9ac418cab1145b96dd6e9cbd9
  • Triggering command: /usr/bin/php8.3 /usr/bin/php8.3 -n -c /tmp/PLAbBE /usr/bin/composer install --no-interaction --prefer-dist (http block)
• https://api.github.com/repos/sebastianbergmann/object-enumerator/zipball/202d0e344a580d7f7d04b3fafce6933e59dae906
  • Triggering command: /usr/bin/php8.3 /usr/bin/php8.3 -n -c /tmp/PLAbBE /usr/bin/composer install --no-interaction --prefer-dist (http block)
• https://api.github.com/repos/sebastianbergmann/object-reflector/zipball/24ed13d98130f0e7122df55d06c5c4942a577957
  • Triggering command: /usr/bin/php8.3 /usr/bin/php8.3 -n -c /tmp/PLAbBE /usr/bin/composer install --no-interaction --prefer-dist (http block)
• https://api.github.com/repos/sebastianbergmann/type/zipball/462699a16464c3944eefc02ebdd77882bd3925bf
  • Triggering command: /usr/bin/php8.3 /usr/bin/php8.3 -n -c /tmp/PLAbBE /usr/bin/composer install --no-interaction --prefer-dist (http block)
• https://api.github.com/repos/sebastianbergmann/version/zipball/c51fa83a5d8f43f1402e3f32a005e6262244ef17
  • Triggering command: /usr/bin/php8.3 /usr/bin/php8.3 -n -c /tmp/PLAbBE /usr/bin/composer install --no-interaction --prefer-dist (http block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)