审查代码：开发人员的必修课

最开始接触代码审查或许可以谈到大学的时候，当时教我们C语言的老师让我们同学间互相检查布置的课后作业的代码的问题，检查出来有问题的可以加学分😄。当时也只是为了学分，没有想太多。后来去公司实习后又一次接触到了代码审查，这一次就要正规得多了，不仅仅是代码没有问题，还要符合一定规范等等要求。代码审查确实会耗去不少时间，但是这也是一个了解别人代码风格的机会，可以从中学习到不少东西。毕竟少有人能耐得下性子去读好的框架等的源代码🤣。因此，如果有公司团队提供代码审查，这无疑是一个值得信赖的团队。所谓三人行必有我师，审查别人的代码，也是学习的过程。

发现常见错误

代码审查的最基础的就是对于一些常见的错误要指出，对于粗心大意的人或者不熟悉对应代码的新人或者说一些容易被忽略的问题。通过这一步能够进行避免。因此，在审查任何代码时，首先要做的就是找出我们开发人员经常忽略的常见错误。

例子1：

public class OrderProcessor
{
    private readonly List<Order> _orders;

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

    public void ProcessOrders()
    {
        foreach (var order in _orders)
        {
            if (order.Status == "Paid")
            {
                order.Status = "Shipped";
                Console.WriteLine($"Order {order.Id} is now Shipped.");
            }
            else if (order.orderStatus== "Shipped")
            {
                order.Status = "Completed";
                Console.WriteLine($"Order {order.Id} is now Completed.");
            }
        }
    }
}

public class Order
{
    public int order_id { get; set; }
    public string orderStatus { get; set; }
    public decimal TotalAmount{ get; set; }
}

你能发现上面的代码的一些问题吗？

• ProcessOrder 函数不包含任何 try-catch 块。如果出了差错，应用程序就会崩溃，而如果没有适当的日志记录，我们将无法知道崩溃的原因（这一点在历史遗留的代码中进行迭代尤其重要，笔者负责过一个遗留的系统，如果往里面迭代新的功能，不加对应的异常处理，稍有不慎就会影响其他功能）
• 在这里，我们使用硬编码值进行状态比较，例如，我们检查订单状态是否为 "已付款"，然后将其标记为 "已发货"。这很容易出错。（所有状态、类型或其他已知可以枚举的字段，统一维护是最好的选择。遍地开花是大忌。）
• 我们没有遵循任何惯例。order_id 是蛇形大小写，而 orderStatus 是驼峰形大小写，TotalAmount 是大写字母。这是不一致的，会使代码变得混乱。（统一风格，对于后续的维护还是迭代等都有一定的好处。毕竟没人想看到风格各异的代码是吧🤣）

例子2：

public class OrderProcessor
{
    public void GetCustomerOrderDetails(Customer customer)
    {
        if (customer.Orders.Count > 0) 
        {
            foreach (var order in customer.Orders)
            {
                Console.WriteLine($"Order ID: {order.OrderId}, Amount: {order.Amount}");
            }
        }
        else
        {
            Console.WriteLine("No orders found.");
        }
    }
}

• 上面的代码咋的一看好像并没有神门问题，但是，作为开发人员，我们经常会忽略添加基本的空值检查，尤其是当我们必须更快地发布产品时。这有可能导致意想不到的错误。

• 进一步思考，如果这是一个前端或网络应用程序接口，我会在进入实际逻辑之前检查验证是否到位。

例子3：

public class OrderService
{
    private readonly PaymentService _paymentService;

    public OrderService()
    {
        _paymentService = new PaymentService();
    }

    public void ProcessOrder(Order order)
    {
        _paymentService.ProcessPayment(order);
    }
}

再看看这一段代码呢，你能看出什么问题？是的，耦合度太深了。系统设计都讲究高内聚，低耦合。因此衍生了DDD设计模式等。特别是对于大型系统、复杂系统。如果模块之间耦合太深，后续不便于拓展和迭代。每一次修改都可能会伤筋动骨。而对于细化到代码层面亦是如此，有时候功能的实现并不是首要考虑的。比如上面的代码，功能很简单，但是OrderService 与 PaymentService 紧密耦合。很难孤立地测试 OrderService，也不能轻易地用模拟或不同的实现来替换 PaymentService。

例子4：

public double CalculateTax(double income)
{
    if (income <= 50000)
        return income * 0.10; // 10% tax for income <= 50,000

    if (income <= 100000)
        return income * 0.15; // 15% tax for income between 50,001 and 100,000

    if (income <= 200000)
        return income * 0.20; // 20% tax for income between 100,001 and 200,000

    return income * 0.30; // 30% tax for income > 200,000
}

和这段代码类似的代码我曾经在遗留的老项目中见过不止一处。对应功能也已经进行了测试，并在目前的项目结构下运行良好。但是，当你看到的第一眼会不会感觉到奇怪？ 是的，太多常量数字了，虽然有注释能够让人明白其意思。但是如果缺失了文档对应的需求更改追溯，更甚至于换了几波人，这个数字变化了若干次。如何去追溯呢？（更甚至注释和数字不对版，如何去判断？） 这些都是增加其他同事接受此代码，会更他们造成费解的地方。最好是完全删除神奇数字，将这些值集中到配置文件、数据库或任何中心位置，这样就可以在不修改代码的情况下轻松更新。如果有修改需求，甚至有必要记录一个更改日志。便于后续此段代码出错追溯问题。

例子5：

public class DiscountCalculator
{
    public double CalculateDiscount(double amount, double discountPercentage)
    {
        double discount = amount * discountPercentage;
        double discountedPrice = amount - discount;
        return discountedPrice;
    }

    public double ApplyDiscount(double amount, double discountPercentage)
    {
        double discount = amount * discountPercentage;
        double discountedPrice = amount - discount;
        return discountedPrice;
    }
}

这个例子也是一个可能经常会出现的例子。是的，重复逻辑编写。同一逻辑在多个地方使用。这意味着，如果逻辑发生变化，我们就必须手动更改所有地方的逻辑。因此，对于重复出现多次的代码，如有必要请抽取出来，便于维护。

例子6：

这个例子不好列举，这里就没有写出来了。简单点说就是不要过度设计，额外的代码，会让事情变得更加复杂。我们的目标应该是保持代码简单、清晰，并专注于手头的实际任务，而不是那些可能永远不会发生的事情。因此，与其猜测以后可能需要什么，我们不如精打细算，只关注现在需要什么。

注意事项

• 在审查代码时保持专业和尊重是非常重要的。良好的团队环境要成员之间互相尊重。或许这个错误在你看来很好笑，但或许别人只是太过年轻亦或粗心。目的是改进代码，而不是批评他人。

• 要有建设性，与其简单地说 "这样做是不对的"，不如这样说："我认为我们这样做可能会更好，因为......"。这会帮助对方学习和成长。

• 提出问题：不要假设任何事情。如果你有什么不明白的地方，最好提出问题，比如 "你为什么用这个？这样的回答可能会给你提供足够的细节，你可以更好地复习。

• 清晰具体：反馈应该直接了当。不要说 "这看起来不太好"。而不是说 "这看起来不太好"，而是要指出哪里不好，需要怎么做。

• 保持平衡：尽量平衡你的反馈意见，指出代码中哪些地方做得好。如果有做得好的地方，请予以肯定。这不仅是为了发现问题，也是为了表扬好的做法。

• 保持开放的心态：对不同的方法持开放态度。你可能有自己的做事方法，但其他人的选择可能有合理的理由。保持对话的合作性，而不是防御性。

• 要有耐心：并非每个人的经验水平都一样。如果你看到一个错误或一些基本的东西被忽视了，请花点时间解释清楚，不要显得居高临下。帮助他人学习，而不仅仅是指出错误。

• 不要吹毛求疵：避免过度分析对功能影响不大的小细节。只要是高质量的代码就足够了。

• 个人偏见：有时我们会有自己的偏见。例如，你喜欢使用空格，但开发人员却到处使用制表符。我们应该忽略这些偏见，进行有意义的审查。

• 尊重时间：这一点不容商量。每个人都有自己的最后期限。给出周到而简洁的反馈，以便他们能在规定时间内执行。

小结

代码审查并不可怕，它可以成为学习、成长和提高项目整体质量的绝佳机会。上面列举了一些常见的例子和审查时候的注意事项。不同公司，不同团队，不同的人可能都有自己的一套规范。取长补短，找准适合自己的定位，融入到当前的团队中去，便是你需要做的。加油💪🏻！