PHP代码审查：资深开发者会说什么以及如何避免出丑

引言：为什么你的代码总被要求重写？

你写好了代码。它能运行。你为自己感到骄傲。但代码审查一过，资深开发者留下了15条评论，其中一半以“那如果……”或“我们把这个重写成……”开头。熟悉吗？

PHP代码审查不是吹毛求疵。这是关于项目的生存。资深开发者看的不仅仅是“能运行/不能运行”，而是代码在负载下的表现、其他开发者如何维护它，以及半年后你要花多少时间来找bug。

在本文中，我们将分析你在经验丰富的PHP开发者的代码审查中最常听到的意见。我会展示真实的代码示例并解释如何修复它们。阅读后，你将能预见80%的评审意见，并写出拿得出手的代码。

1. “魔法数字”和字符串：代码应该不言自明

资深开发者首先会注意到的是散落在代码中的难以理解的数字和字符串。魔法数字是一种破坏可读性的反模式。

糟糕的例子

if ($user->getAge() > 18) {\n    $discount = $price * 0.1;\n}

资深开发者会说什么？

\n
• 为什么是18？这是成年年龄还是折扣门槛？
\n
• 为什么是0.1？这是10%吗？如果折扣变了呢？
\n

好的例子

const ADULT_AGE = 18;\nconst DISCOUNT_RATE = 0.1; // 10%\n\nif ($user->getAge() > ADULT_AGE) {\n    $discount = $price * DISCOUNT_RATE;\n}

现在代码自己讲述了故事。资深开发者会认可：常量可以复用，在一个地方修改，而且不需要注释。

2. 缺少类型声明：PHP 8 不是动态类型化的借口

PHP是一种弱类型语言，但这并不意味着你应该像2005年那样写代码。强类型可以防止难以捕获的bug。

糟糕的例子

function calculateTotal($items, $tax) {\n    $total = 0;\n    foreach ($items as $item) {\n        $total += $item['price'] * $item['quantity'];\n    }\n    return $total * (1 + $tax);\n}

资深开发者的意见

\n
• $items是什么？数组？对象？集合？
\n
• 元素的结构是什么？为什么没有DTO？
\n
• 如果传入字符串形式的$tax "0.2"呢？会发生字符串拼接吗？
\n

好的例子

declare(strict_types=1);\n\nclass OrderItem {\n    public function __construct(\n        public readonly float $price,\n        public readonly int $quantity\n    ) {}\n}\n\nfunction calculateTotal(array $items, float $taxRate): float {\n    $total = 0.0;\n    foreach ($items as $item) {\n        if (!$item instanceof OrderItem) {\n            throw new InvalidArgumentException('Expected OrderItem');\n        }\n        $total += $item->price * $item->quantity;\n    }\n    return $total * (1 + $taxRate);\n}

资深开发者会说：“干得好，你使用了DTO、readonly属性和强类型。代码是可预测的。”这是PHP 8+的最佳实践。

3. 深层嵌套和提前返回（Early Return）

当你在一个方法中看到5层if...else嵌套时，资深开发者会抱头。这样的代码无法阅读、测试和调试。

糟糕的例子

public function processOrder($order) {\n    if ($order->isPaid()) {\n        if ($order->hasStock()) {\n            if ($order->isShippable()) {\n                // 50行逻辑\n                return $this->ship($order);\n            } else {\n                throw new \\Exception('Order cannot be shipped');\n            }\n        } else {\n            throw new \\Exception('Out of stock');\n        }\n    } else {\n        throw new \\Exception('Order not paid');\n    }\