每一次AI生成的代码之后我都会对其进行review,时间长了之后发现有几类修改是常常需要我回滚或者介入进行二次修改的。这类生成的代码看上去漂亮,但仔细看来没有存在的必要。这里对它们进行一个总结归纳。

味同嚼蜡的注释

AI总是不厌其烦的为我生成注释,有函数级别的,比如下面这种:

/// <summary>
/// 向指定 URL 发送 GET 请求并返回响应内容的字符串。
/// </summary>
/// <param name="client">用于发送请求的 <see cref="HttpClient"/> 实例。</param>
/// <param name="url">请求的目标 URL。</param>
/// <returns>响应体的原始字符串,通常为 JSON。</returns>
/// <exception cref="HttpRequestException">请求失败或响应状态码表示失败时抛出。</exception>
/// <exception cref="TaskCanceledException">请求被取消或超时时抛出。</exception>
static async Task<string> GetApiResponseAsync(HttpClient client, string url)
{
    var response = await client.GetAsync(url);
    response.EnsureSuccessStatusCode();
    return await response.Content.ReadAsStringAsync();
}

也有行内级别的,比如这种:

// 格式化输出 JSON(缩进便于阅读)
using var doc = JsonDocument.Parse(json);

大部分时候AI生成的代码都附带着各种各样的注释,给我一种买一斤送一斤赚到的错觉。但我其实是对它生成的注释感到头疼的,因为实在毫无必要。

注释当然有用,但前提是它是一则好的注释。可据我观察大部分程序员手写的注释都不算好,原因不外乎以下两点:

  • 从心态上来说,注释被大部分程序员拿来当作给代码“擦屁股”的工具——例如这个变量名或者函数名我不知道改如何取,或者这个函数的职责太多以及数过于复杂,再或者我实在没辙了,那我就用注释解释一下吧——在这一类场景下对代码进行重构比写注释更能达到效果。
  • “写注释”这件事的本质其实是沟通技巧的一种显现,如果这个人平时和同事沟通都费劲那么也别指望着他注释能够写的多好。更何况注释是一种将技术语言用非技术语言呈现的手段,本身就颇具难度——我想要表达的是,要把注释写好也是需要技巧的,试想一下用几句话就把几行代码的用途、上下文解释清楚其实是不简单的。而程序员更擅长沟通还是写代码?显然是后者。

在我看来最好的注释无非就是代码本身,代码自己就是对自己的最好诠释。这里我引用《代码整洁之道》中的一个例子,你不妨看看是Option A的表达还是Option B的表达更好?

// ------ Option A ------
// Check to see if the employee is eligible for full benefits
if ((employee.flags & HOURLY_FLAG) &&
(employee.age > 65))

// ------ Option B ------
if (employee.isEligibleForFullBenefits())

现在再回过头看我在本小节开头的AI为我生成的代码的例子,其实他们完全没有必要存在, JsonDocument.Parse(json); 本身已经足够表意了。

但我不难理解为什么AI热衷于添加日志,因为本质上它是一种平均意志的体现。我自己在相当长的一段时间内也是把注释当作某种代码创口贴在使用,直到读到《代码整洁之道》。如果用于训练AI模型的代码库总是穿插着各种各样的注释,那么我们又怎么能指望AI生成的代码会完全摒弃注释呢。

IMG_4570_3.jpg

AI生成的注释还有另一个风险:注释通常是函数第一次被创建时自动生成的,后续如果有其他开发者接手了代码库的维护工作并修改了实现,可能他会忘记也同时对注释中的内容也进行对应的修改。避免这类问题的风险解决方案也并不复杂,可以在代码持续集成流水线中引入AI agent来根据提交的代码对注释做相应修改,这部分实现可以参考Cursor官方提供的有关如使用Cursor CLI来更新文档的示例

本文中的注释只是为了更好的与原文呼应,便于读者找到对应的代码。

万能的静态方法

我发现AI特别喜欢将交给它重构的代码抽离为静态方法,注意下面这段代码中使用httpClient用于发送请求并接收返回的部分:


using var httpClient = new HttpClient();
try
{
    Console.WriteLine("正在请求 Random User API...\n");
    
    // ------ 👇 注意下述三行代码 ------
    var response = await httpClient.GetAsync(apiUrl);
    response.EnsureSuccessStatusCode();
    var json = await response.Content.ReadAsStringAsync();

    using var doc = JsonDocument.Parse(json);
    var formatted = JsonSerializer.Serialize(doc, new JsonSerializerOptions
    {
        WriteIndented = true
    });
    Console.WriteLine("API 返回结果:\n");
    Console.WriteLine(formatted);
}
catch (HttpRequestException ex)
{
    Console.WriteLine($"请求失败: {ex.Message}");
}

在重构中会被AI抽为一个静态方法:

// 新的静态方法
static async Task<string> GetApiResponseAsync(HttpClient client, string url)
{
    var response = await client.GetAsync(url);
    response.EnsureSuccessStatusCode();
    return await response.Content.ReadAsStringAsync();
}

// 原代码部分:
try
{
    Console.WriteLine("正在请求 Random User API...\n");
    // ------- 👇 被替换部分 ------
    var json = await GetApiResponseAsync(httpClient, apiUrl

    using var doc = JsonDocument.Parse(json);
		//...

从实现上来说它没问题,这几行代码符合静态方法的特征,例如被抽离出来的代码并不依赖类实例化之后的变量,执行也不会带来副作用。同时可以想象这也是重构的最快方式。

但如果是真正有C#背景的程序员来负责这部分代码的重构,他绝不会选择这样的方式,常见的模式是:

  • 他会创建一个client角色类型的类来封装这部分代码。在编码中client通常承担外部服务的调用职责。例如访问数据库、发送网络请求等等
  • 在实现上述client类时,会遵循“接口”-“实现”的模式

具体代码如下:

// 接口部分
public interface IApiClient
{
    Task<string> GetApiResponseAsync(string url);
}

// 实现部分
public class HttpApiClient : IApiClient
{
    private readonly HttpClient _httpClient = new();

    /// <inheritdoc />
    public async Task<string> GetApiResponseAsync(string url)
    {
        var response = await _httpClient.GetAsync(url);
        response.EnsureSuccessStatusCode();
        return await response.Content.ReadAsStringAsync();
    }
}

在我看来client的角色的存在倒是其次,重要的接口的出现。只有它的存在才让我们可以实现多态,更好的做到依赖注入。举个非常简单的例子,接口的出现可以让我们轻易在单元测试中对GetApiResponseAsync的实现做替换,来模拟各种情况的发生

// 在测试中模拟正常返回
const string rawJson = @"{""results"":[],""info"":{""seed"":""x"",""version"":""1.4""}}";
var mockApiClient = new Mock<IApiClient>();
mockApiClient
    .Setup(m => m.GetApiResponseAsync(It.IsAny<string>()))
    .ReturnsAsync(rawJson);
    
//......
      
// 在测试中模拟方法调用出错情况
var mockApiClient = new Mock<IApiClient>();
mockApiClient
    .Setup(m => m.GetApiResponseAsync(It.IsAny<string>()))
    .ThrowsAsync(new HttpRequestException("Connection refused"));
using var output = new StringWriter();

聊胜于无的单元测试

AI习惯于生成大量的单元测试,以下面这段实现代码为例

public static async Task RunAsync(IApiClient apiClient, TextWriter output, string url)
{
    try
    {
        await output.WriteLineAsync("正在请求 Random User API...\n");
        var json = await apiClient.GetApiResponseAsync(url);

        using var doc = JsonDocument.Parse(json);
        var formatted = JsonSerializer.Serialize(doc, new JsonSerializerOptions
        {
            WriteIndented = true,
        });
        await output.WriteLineAsync("API 返回结果:\n");
        await output.WriteLineAsync(formatted);
    }
    catch (HttpRequestException ex)
    {
        await output.WriteLineAsync($"请求失败");
    }
		//...
}

这段代码中我们已经按照上一小节的方案使用了IApiClient接口来注入依赖的apiClient。注意在代码中有一段捕获HttpRequestException的逻辑。如果让AI针对这段出错处理逻辑生成测试,会得到的结果大致如下:

[Fact]
public async Task RunAsync_WhenApiThrowsBadRequest_Writes400Message()
{
    var mockApiClient = new Mock<IApiClient>();
    mockApiClient
        .Setup(m => m.GetApiResponseAsync(It.IsAny<string>()))
        .ThrowsAsync(new HttpRequestException("Request failed", 
		        null, HttpStatusCode.BadRequest));

    using var output = new StringWriter();
    await App.RunAsync(mockApiClient.Object, output, "https://example.com/api");

    var text = output.ToString();
    Assert.Contains("请求失败", text);
}

[Fact]
public async Task RunAsync_WhenApiThrowsNotFound_WritesRequestFailedMessage()
{
    var mockApiClient = new Mock<IApiClient>();
    mockApiClient
        .Setup(m => m.GetApiResponseAsync(It.IsAny<string>()))
        .ThrowsAsync(new HttpRequestException("Request failed", 
		        null, HttpStatusCode.NotFound));

		//......
}

[Fact]
public async Task RunAsync_WhenApiThrowsInternalServerError_WritesRequestFailedMessage()
{
    var mockApiClient = new Mock<IApiClient>();
    mockApiClient
        .Setup(m => m.GetApiResponseAsync(It.IsAny<string>()))
        .ThrowsAsync(new HttpRequestException("Request failed", 
		        null, HttpStatusCode.InternalServerError));

		//......
}

简单来说,AI会例举出HttpRequestException衍生出每一种情况并针对性的写出测试。所以在上面看到了它模拟接口返回404、500、400状态码的情况。但这完全是没有必要的,因为catch代码块里并没有针对不同的状态码给予不同的处理逻辑,生成更多的测试用例既无法减少代码的出错概率,也无法提升各种代码覆盖率。

是好是坏看你站在什么样的角度上看待这个问题了,如果你现在的编码模式已经彻底转变为“AI生成、AI验证、AI修复”这类由AI端到端的交付模式,这其实不算什么坏事。毕竟额外生成的测试也不费成本,测试出错了还是由AI买单,人可以彻底置身事外。但如果在你的开发模式中人依然处于主导地位,那很明显AI生成的测试会加重维护成本——例如程序员需要对AI生成的代码进行review的理解和时间成本就增加了。

我是追求测试覆盖率的反对者,因为盲目的追求测试覆盖率会让大量时间花费在小概率事件上,性价比极低。单元测试是一门平衡的艺术,过多的投入是浪费,过少投入会带来风险。我倾向于交给需求的开发者自行决定,他需要问自己的一个问题是:写到何种程度的单元测试能给他足够信心允许代码被部署到线上运行?或者你也可以参考我编写单元测试的策略:1)必须覆盖明确的产品需求;2)覆盖潜在风险,例如可能会导致程序崩溃的非法输入;3)在允许时间范围内继续丰富测试

牺牲可读性

下面的例子可能不够准确,但我相信你应该可以明白我的意思。

首先看下面这段代码,我定义了一个名为IDatabaseClient的接口,再在两个Client中对接口中的Query方法进行了实现

// 定义了数据库客户端的统一契约(接口)。
public interface IDatabaseClient
{
    Task Query(string queryStatement);
}

// 代表 PostgreSQL 客户端
public class PostgresClient: IDatabaseClient
{
    public async Task Query(string queryStatement)
    {
        Console.WriteLine("PostgresClient DB Query Completed");
    }
}

// 代表 SQLServer 客户端
public class SQLServerClient: IDatabaseClient
{
    public async Task Query(string queryStatement)
    {
        Console.WriteLine("SQLServerClient DB Query Completed");
    }
}

两个实现的Query方法中我都添加了一行日志打印语句,采用的某种通用的格式XXXClient DB Query Completed,在实际代码中我会用该日志表明数据库查询完毕。

如果你让AI对重复的日志内容进行优化,有大概率会得到下面的这样的结果,先看UML类图:

svgviewer-png-output.jpg

AI为我添加了一个名为DatabaseClientBase的抽象类,PostgresClientSQLServerClient不再直接实现IDatabaseClient接口,而是选择继承DatabaseClientBase类,该抽象类的实现如下:

public abstract class DatabaseClientBase : IDatabaseClient
{
    private string ClientName => GetType().Name;

    protected void LogMethodCalled([CallerMemberName] string? methodName = null) =>
        Console.WriteLine($"{ClientName} {methodName} method called");

    public abstract Task Query(string queryStatement);
}

其中最为关键的就是LogMethodCalled方法,之所以AI选择让PostgresClient继承DatabaseClientBase类,是因为它想让其调用其中的LogMethodCalled方法来避免重复日志内容的出现:

public class PostgresClient : DatabaseClientBase
{
    public override async Task Query(string queryStatement)
    {
        LogMethodCalled();
    }
}

从实现上来说,AI把日志代码封装的非常好,它自动为你在日志语句中注入了类的名称以及调用方法的名——但我却不认为这个是一个好的实践。

如此生成出来的日志会让开发者调试代码变得更难,从而违背了日志出现的意义。可以想象如果开发者在线上环境看到PostgresClient Query method called字样的日志输出后,他是无法通过简单的复制粘贴的全局搜索到这条日志所在的代码位置的,因为该日志一个拼接后的结果。他只能将日志按单词拆解,再按照排除法来确认日志究竟是在什么位置出现。并且从开发者的角度上看,明明只是一个简单的单行日志输出,我却为其新增了新的抽象类,改变了类的继承结构,听上去这似乎有些本末倒置。

有一类代码我会觉得脏就脏了,无所谓的。因为我知道它是一次性,我知道它不会对后期的项目的继续维护产生影响。

总结

解决它们的方法非常简单。以Cursor为例,你可以通过配置cursor rules来规范AI的行为,还可以使用sub agent来对修改进行review。这两类工具背后的本质其实是一样的:将你的想法暴露出去,告诉它们你想要什么,而不是让AI决定你需要什么。

所以说到底我的上述观点不重要,你同不同意我的观点也不重要,重要的是你的观点是什么?