代码审查实战：6个常见案例教你避坑与提升代码质量

我从事代码审查已有相当长的一段时间，并总结出了一些常见模式，这些模式帮助我更好地进行代码审查。根据我的经验，我整理了一份在审查任何拉取请求时关注的重点清单。

在本文中，我们将一起进行代码审查，并学习审查时需要注意的事项。我们还将讨论如何以专业和尊重的方式进行代码审查。

审查 1：发现常见错误

在审查代码时，我首先关注的是识别开发者常忽略的常见错误。让我们通过以下代码来发现一些常见错误和反模式：

代码语言：javascript代码运行次数：0运行复制

public classOrderProcessor
{
    privatereadonlyList<Order> _orders;

    publicOrderProcessor(List<Order> orders)
    {
        _orders = orders;
    }

    publicvoidProcessOrders()
    {
        foreach(var order in _orders)
        {
            if(order.orderStatus =="Paid")
            {
                order.orderStatus ="Shipped";
                Console.WriteLine($"Order {order.order_id} is now Shipped.");
            }
            elseif(order.orderStatus =="Shipped")
            {
                order.orderStatus ="Completed";
                Console.WriteLine($"Order {order.order_id} is now Completed.");
            }
        }
    }
}

publicclassOrder
{
    publicint order_id {get;set;}
    publicstring orderStatus {get;set;}
    publicdecimal TotalAmount{get;set;}
}

你发现代码中的问题了吗？

是的！这段代码存在多个问题：

1. 缺乏异常处理：ProcessOrders 方法没有 try-catch 块，如果出错会导致应用程序崩溃且无日志记录。
2. 硬编码状态值：直接使用字符串（如 "Paid" 和 "Shipped"）进行状态比较，容易出错。
3. 命名不一致：Order 类的属性命名混乱（如 order_id 是蛇形命名，orderStatus 是驼峰命名，TotalAmount 是帕斯卡命名）。

修复后的代码：

代码语言：javascript代码运行次数：0运行复制

using System;
usingSystem.Collections.Generic;

publicclassOrderProcessor
{
    privatereadonlyList<Order> _orders;

    publicOrderProcessor(List<Order> orders)
    {
        _orders = orders;
    }

    publicvoidProcessOrders()
    {
        foreach(var order in _orders)
        {
            try
            {
                // 使用枚举进行状态比较
                if(order.Status == OrderStatus.Paid)
                {
                    order.Status = OrderStatus.Shipped;
                    Console.WriteLine($"Order {order.Id} is now Shipped.");
                }
                elseif(order.Status == OrderStatus.Shipped)
                {
                    order.Status = OrderStatus.Completed;
                    Console.WriteLine($"Order {order.Id} is now Completed.");
                }
                else
                {
                    Console.WriteLine($"Order {order.Id} has an unknown status: {order.Status}");
                }
            }
            catch(Exception ex)
            {
                // 正确处理并记录异常
                Console.WriteLine($"Error processing order {order.Id}: {ex.Message}");
                throw ex;
            }
        }
    }
}

publicclassOrder
{
    // 保持一致的命名规范（帕斯卡命名）
    publicint Id {get;set;}
    publicOrderStatus Status {get;set;}
    publicdecimal TotalAmount {get;set;}
}

// 使用枚举存储状态值
publicenumOrderStatus
{
    Paid,
    Shipped,
    Completed
}

---

审查 2：最常见的遗漏

你能审查以下代码吗？