PHP版Gilded Rose重构：Switch替换If后测试失败排查

问题描述 #

正在进行PHP版Gilded Rose重构练习，已针对原版GildedRose.php编写单元测试且全部通过。尝试将原代码中40行if语句替换为switch-case结构以提升可读性，但出现错误导致测试不通过，需排查问题。

单元测试示例 #

public function testAgedBrieAtBeginning(): void          //  +1 quality till sell_in = 0 than +2 quality
{
    $items = [new Item('Aged Brie', 2, 0)];
    $gildedRose = new GildedRose($items);
    $gildedRose->updateQuality();
    $this->assertEquals($items[0]->sell_in, 1);
    $this->assertEquals($items[0]->quality, 1);
}

public function testAgedBrieWhenSellInZero(): void
{
    $items = [new Item('Aged Brie', 0, 2)];
    $gildedRose = new GildedRose($items);
    $gildedRose->updateQuality();
    $this->assertEquals($items[0]->sell_in, -1);
    $this->assertEquals($items[0]->quality, 4);
}

public function testAgedBrieDontIncreaseMaximum(): void       // stand at 50 quality
{
    $items = [new Item('Aged Brie', -28, 50)];
    $gildedRose = new GildedRose($items);
    $gildedRose->updateQuality();
    $this->assertEquals($items[0]->sell_in, -29);
    $this->assertEquals($items[0]->quality, 50);
}

修改后的GildedRose.php代码 #

<?php

declare(strict_types=1);

namespace GildedRose;

final class GildedRose
{
/**
 * @var Item[]
 */
private $items;

public function __construct(array $items)
{
    $this->items = $items;

}

public function updateQuality(): void
{
    switch ($this->items) {
        case 'Normal':
            $this->tickNormal();
            return;
        case 'Aged Brie':
            $this->tickAgedBrie();
            return;
        case 'Sulfaras, Hand of Ragnaros':
            $this->tickSulfuras();
            return;
        case 'Backstage passes to a TAFKAL80ETC concert':
            $this->tickBackstage();
            return;
    }

}

private function tickNormal(): void  // for Dexterity, Elixir & Conjured
{
    $this->sell_in--;

    if($this->quality == 0) {
        return;
    }

    $this->quality--;
    if ($this->sell_in <= 0) {
        $this->quality--;
    }
}
private function tickAgedBrie()
{
    $this->sell_in -= 1;

    if($this->quality >= 50) {
        return;
    }

    $this->quality += 1;
    if ($this->sell_in <= 0) {
        $this->quality += 1;
    }
}

private function tickSulfuras() : void
{

}

private function tickBackstage() : void
{
    $this->sell_in -= 1;

    if ($this->quality >= 50) {
        return;
    }
    if ($this->sell_in < 0) {
        $this->quality = 0;
        return;
    }

    $this->quality += 1;
    if ($this->sell_in < 10) {
        $this->quality += 1;
    }
    if ($this->sell_in < 5) {
        $this->quality += 1;
    }
}

}

问题分析与修复方案 #

你的代码存在几个核心错误，导致测试失败：

1. 未遍历物品数组 #

原代码逻辑是遍历每个Item实例进行属性更新，但你现在的updateQuality方法直接对整个$this->items数组做switch判断，数组无法匹配任何字符串case，导致所有物品都未被处理。

2. 错误访问属性 #

GildedRose类本身没有sell_in和quality属性，这两个属性属于Item类的实例。你在tick方法中直接使用$this->sell_in，会触发"未定义属性"的致命错误。

3. switch判断对象错误 #

应该针对单个Item的name字段做switch判断，而非整个物品数组。

4. 方法缺失返回类型 #

tickAgedBrie方法未声明返回类型，在strict_types=1模式下会引发类型错误。

5. Normal物品逻辑错误 #

原代码中"Normal"是默认情况（非特殊物品），而非物品名为"Normal"，你的case 'Normal'永远无法匹配到实际物品。

修复后的核心代码示例 #

public function updateQuality(): void
{
    foreach ($this->items as $item) {
        switch ($item->name) {
            case 'Aged Brie':
                $this->tickAgedBrie($item);
                break;
            case 'Sulfaras, Hand of Ragnaros':
                $this->tickSulfuras($item);
                break;
            case 'Backstage passes to a TAFKAL80ETC concert':
                $this->tickBackstage($item);
                break;
            default:
                // 处理普通物品和Conjured物品
                $this->tickNormal($item);
                break;
        }
    }
}

private function tickNormal(Item $item): void
{
    $item->sell_in--;
    $decrement = str_contains($item->name, 'Conjured') ? 2 : 1;

    if ($item->quality <= 0) {
        return;
    }

    $item->quality -= $decrement;
    if ($item->sell_in <= 0) {
        $item->quality -= $decrement;
    }
    // 确保quality不低于0
    $item->quality = max(0, $item->quality);
}

private function tickAgedBrie(Item $item): void
{
    $item->sell_in--;

    if ($item->quality >= 50) {
        return;
    }

    $item->quality++;
    if ($item->sell_in <= 0) {
        $item->quality++;
    }
    // 确保quality不超过50
    $item->quality = min(50, $item->quality);
}

private function tickBackstage(Item $item): void
{
    $item->sell_in--;

    if ($item->quality >= 50) {
        return;
    }

    if ($item->sell_in < 0) {
        $item->quality = 0;
        return;
    }

    $item->quality++;
    if ($item->sell_in < 10) {
        $item->quality++;
    }
    if ($item->sell_in < 5) {
        $item->quality++;
    }
    $item->quality = min(50, $item->quality);
}

private function tickSulfuras(Item $item): void
{
    // Sulfuras无需更新sell_in和quality
}

内容的提问来源于stack exchange，提问作者massariH