Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Copilot wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eb55498
Initial plan
Copilot Dec 17, 2025
c05608c
Add comprehensive test suite for all business rules
Copilot Dec 17, 2025
c6162f2
Refactor: Extract constants, simplify conditionals, and improve metho…
Copilot Dec 17, 2025
4f41134
Refactor: Convert if-elseif-else to guard clauses in updateQualityAft…
Copilot Dec 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 103 additions & 45 deletions src/GildedRose.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,19 @@

class GildedRose
{
private const AGED_BRIE = 'Aged Brie';
private const BACKSTAGE_PASSES = 'Backstage passes to a TAFKAL80ETC concert';
private const SULFURAS = 'Sulfuras, Hand of Ragnaros';

private const MAX_QUALITY = 50;
private const MIN_QUALITY = 0;

private const QUALITY_INCREASE = 1;
private const QUALITY_DECREASE = 1;

private const BACKSTAGE_FIRST_THRESHOLD = 11;
private const BACKSTAGE_SECOND_THRESHOLD = 6;

/**
* @var Item[]
*/
Expand All @@ -20,52 +33,97 @@ public function __construct(array $items)
public function updateQuality(): void
{
foreach ($this->items as $item) {
if ($item->name != 'Aged Brie' && $item->name != 'Backstage passes to a TAFKAL80ETC concert') {
if ($item->quality > 0) {
if ($item->name != 'Sulfuras, Hand of Ragnaros') {
$item->quality = $item->quality - 1;
}
}
} else {
if ($item->quality < 50) {
$item->quality = $item->quality + 1;
if ($item->name == 'Backstage passes to a TAFKAL80ETC concert') {
if ($item->sellIn < 11) {
if ($item->quality < 50) {
$item->quality = $item->quality + 1;
}
}
if ($item->sellIn < 6) {
if ($item->quality < 50) {
$item->quality = $item->quality + 1;
}
}
}
}
}

if ($item->name != 'Sulfuras, Hand of Ragnaros') {
$item->sellIn = $item->sellIn - 1;
}

if ($item->sellIn < 0) {
if ($item->name != 'Aged Brie') {
if ($item->name != 'Backstage passes to a TAFKAL80ETC concert') {
if ($item->quality > 0) {
if ($item->name != 'Sulfuras, Hand of Ragnaros') {
$item->quality = $item->quality - 1;
}
}
} else {
$item->quality = $item->quality - $item->quality;
}
} else {
if ($item->quality < 50) {
$item->quality = $item->quality + 1;
}
}
}
$this->updateItemQuality($item);
}
}

private function updateItemQuality(Item $item): void
{
if ($this->isSulfuras($item)) {
return;
}

$this->updateQualityBeforeSellIn($item);
$this->decreaseSellIn($item);
$this->updateQualityAfterSellIn($item);
}

private function updateQualityBeforeSellIn(Item $item): void
{
if ($this->isAgedBrie($item)) {
$this->increaseQuality($item);
} elseif ($this->isBackstagePass($item)) {
$this->updateBackstagePassQuality($item);
} else {
$this->decreaseQuality($item);
}
}

private function updateQualityAfterSellIn(Item $item): void
{
if ($item->sellIn >= 0) {
return;
}

if ($this->isAgedBrie($item)) {
$this->increaseQuality($item);
} elseif ($this->isBackstagePass($item)) {
$this->resetQuality($item);
} else {
$this->decreaseQuality($item);

@amlndz amlndz Dec 17, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot revisa esto y cambialo a clausulas de guarda

Copilot AI Dec 17, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}

private function updateBackstagePassQuality(Item $item): void
{
$this->increaseQuality($item);

if ($item->sellIn < self::BACKSTAGE_FIRST_THRESHOLD) {
$this->increaseQuality($item);
}

if ($item->sellIn < self::BACKSTAGE_SECOND_THRESHOLD) {
$this->increaseQuality($item);
}
}

private function increaseQuality(Item $item): void
{
if ($item->quality < self::MAX_QUALITY) {
$item->quality += self::QUALITY_INCREASE;
}
}

private function decreaseQuality(Item $item): void
{
if ($item->quality > self::MIN_QUALITY) {
$item->quality -= self::QUALITY_DECREASE;
}
}

private function decreaseSellIn(Item $item): void
{
$item->sellIn--;
}

private function resetQuality(Item $item): void
{
$item->quality = self::MIN_QUALITY;
}

private function isSulfuras(Item $item): bool
{
return $item->name === self::SULFURAS;
}

private function isAgedBrie(Item $item): bool
{
return $item->name === self::AGED_BRIE;
}

private function isBackstagePass(Item $item): bool
{
return $item->name === self::BACKSTAGE_PASSES;
}
}

Loading