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

当前位置：点晴教程→知识管理交流 →『技术文档交流』

admin

2025年3月22日 19:44 本文热度 145

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

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

审查 1：发现常见错误

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

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;}
}

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

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

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

修复后的代码：

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：最常见的遗漏

你能审查以下代码吗？🧐

public classOrderProcessor
{
    publicvoidGetCustomerOrderDetails(Customer customer)
    {
        if(customer.Orders.Count >0)
        {
            foreach(var order in customer.Orders)
            {
                Console.WriteLine($"Order ID: {order.Id}, Amount: {order.Amount}");
            }
        }
        else
        {
            Console.WriteLine("No orders found.");
        }
    }
}

你发现问题了吗？

没错！缺少空值检查。开发者常常为了快速交付而忽略基础的空值检查，这可能导致意外错误。

修复后的代码：

public classOrderProcessor
{
    publicvoidGetCustomerOrderDetails(Customer customer)
    {
        // 空值检查以避免 NullReferenceException
        if(customer ==null)
        {
            Console.WriteLine("Customer is null.");
            return;
        }

        if(customer.Orders ==null)
        {
            Console.WriteLine("Customer has no orders.");
            return;
        }

        // 现在可以安全访问 Orders 列表
        if(customer.Orders.Count >0)
        {
            foreach(var order in customer.Orders)
            {
                Console.WriteLine($"Order ID: {order.Id}, Amount: {order.Amount}");
            }
        }
        else
        {
            Console.WriteLine("No orders found.");
        }
    }
}

审查 3：反模式

以下代码有什么问题？🤔

public classOrderService
{
    privatereadonlyPaymentService _paymentService;

    publicOrderService()
    {
        _paymentService =newPaymentService();
    }

    publicvoidProcessOrder(Order order)
    {
        _paymentService.ProcessPayment(order);
    }
}

正确！代码存在紧耦合。

OrderService 与 PaymentService 紧耦合，难以独立测试且无法轻松替换为模拟实现。

修复后的代码：

public classOrderService
{
    privatereadonlyIPaymentService _paymentService;

    // 使用依赖注入解耦并提高可测试性
    publicOrderService(IPaymentService paymentService)
    {
        _paymentService = paymentService;
    }

    publicvoidProcessOrder(Order order)
    {
        _paymentService.ProcessPayment(order);
    }
}

审查 4：魔法数字 ✨

准备好审查下一个代码了吗？

某 PR 中，你的同事编写了一个计算税款的方法并请你审查：

public doubleCalculateTax(double income)
{
    if(income <=50000)
        return income *0.10;// 收入 ≤ 50,000 的税率为 10%

    if(income <=100000)
        return income *0.15;// 收入 50,001–100,000 的税率为 15%

    if(income <=200000)
        return income *0.20;// 收入 100,001–200,000 的税率为 20%

    return income *0.30;// 收入 > 200,000 的税率为 30%
}

发现问题了吗？

是的！代码中充斥着魔法数字（硬编码数值）。虽然注释让代码更清晰，但这仍然是糟糕的实现。

修复方案（将数值移至配置文件）：

public doubleCalculateTax(double income,IConfiguration configuration)
{
    double[] incomeSlabs = configuration.GetSection("TaxSettings:IncomeSlabs").Get<double[]>();
    double[] taxRates = configuration.GetSection("TaxSettings:TaxRates").Get<double[]>();

    for(int i =0; i < incomeSlabs.Length; i++)
    {
        if(income <= incomeSlabs[i])
        {
            return income * taxRates[i];
        }
    }

    return income * taxRates.Last();// 默认税率（收入超过所有区间）
}

对应的配置文件：

{
  "TaxSettings":{
    "IncomeSlabs":[50000,100000,200000],// 收入区间阈值
    "TaxRates":[0.10,0.15,0.20,0.30]     // 对应税率
}
}

审查 5：DRY 原则

另一个待审查的代码：

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

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

发现问题了吗？

是的！重复代码。相同的逻辑在多个地方重复，违反了 DRY（不要重复自己）原则。

修复后的代码：

public classDiscountCalculator
{
    publicdoubleCalculateDiscount(double amount,double discountPercentage)
    {
        returnCalculateDiscountAmount(amount, discountPercentage);
    }

    publicdoubleApplyDiscount(double amount,double discountPercentage)
    {
        double discount =CalculateDiscountAmount(amount, discountPercentage);
        return amount - discount;
    }

    privatedoubleCalculateDiscountAmount(double amount,double discountPercentage)
    {
        return amount * discountPercentage;
    }
}

审查 6：YAGNI 原则

需求背景：开发者需要实现基于预定义折扣率的订单折扣逻辑。

提交的代码：

public classOrderProcessor
{
    publicvoidProcessOrder(Order order,double discount)
    {
        decimal discountAmount =CalculateDiscount(order, discount);
        decimal finalAmount = order.Amount - discountAmount;

        Console.WriteLine($"Discount Applied: {discountAmount:C}");

        if(!string.IsNullOrEmpty(order.CouponCode))
        {
            decimal couponDiscount =ApplyCoupon(order.CouponCodeDiscount);
            finalAmount -= couponDiscount;
            Console.WriteLine($"Coupon {order.CouponCode} applied.");
        }
    }

    privatedecimalCalculateDiscount(Order order,double discount)
    {
        return order.Amount * discount;
    }

    privatedecimalApplyCoupon(double couponCodeDiscount)
    {
        return order.Amount * couponCodeDiscount;
    }
}

publicclassOrder
{
    publicint Id {get;set;}
    publicdecimal Amount {get;set;}
    publicdouble CouponCodeDiscount {get;set;}
}

发现问题了吗？

ApplyCoupon() 方法是多余的！开发者添加了优惠券处理逻辑，但需求中并未提及此功能。这违反了 YAGNI（你不需要它）原则。

修复后的代码：

public classOrderProcessor
{
    publicvoidProcessOrder(Order order,decimal discount)
    {
        decimal discountAmount =CalculateDiscount(order, discount);
        decimal finalAmount = order.Amount - discountAmount;

        Console.WriteLine($"Discount Applied: {discountAmount:C}, Final Amount: {finalAmount:C}");
    }

    privatedecimalCalculateDiscount(Order order,decimal discount)
    {
        return order.Amount * discount;
    }
}

publicclassOrder
{
    publicint Id {get;set;}
    publicdecimal Amount {get;set;}
}

代码审查的注意事项

在代码审查中，保持专业和尊重至关重要。以下是我总结的指导原则：

保持尊重
：目标是改进代码，而非批评开发者。
提供建设性反馈
：不要说“这不对”，而是说“如果这样做会更好，因为…”。
多提问
：如果不理解某些代码，直接询问“为什么这样实现？”。
清晰具体
：避免模糊反馈（如“这看起来不好”），明确指出问题。
保持平衡
：既要指出问题，也要肯定代码中的优点。
开放心态
：接受不同的实现方式，保持协作态度。
保持耐心
：耐心解释基础问题，帮助他人成长。
避免吹毛求疵
：不要过度关注对功能影响小的细节。
避免个人偏见
：忽略代码风格差异（如空格 vs 制表符）。
尊重时间
：提供简洁有效的反馈，确保开发者能按时完成任务。

阅读原文：原文链接

该文章在 2025/3/24 16:44:42 编辑过

关键字查询

代码

质量

正在查询...

点晴ERP是一款针对中小制造业的专业生产管理软件系统,系统成熟度和易用性得到了国内大量中小企业的青睐。

点晴PMS码头管理系统主要针对港口码头集装箱与散货日常运作、调度、堆场、车队、财务费用、相关报表等业务管理，结合码头的业务特点，围绕调度、堆场作业而开发的。集技术的先进性、管理的有效性于一体，是物流码头及其他港口类企业的高效ERP管理信息系统。

点晴WMS仓储管理系统提供了货物产品管理,销售管理,采购管理,仓储管理,仓库管理,保质期管理,货位管理,库位管理,生产管理,WMS管理系统,标签打印,条形码,二维码管理,批号管理软件。

点晴免费OA是一款软件和通用服务都免费，不限功能、不限时间、不限用户的免费OA协同办公管理系统。