一天一点代码坏味道（2）

2023-11-15 13:57:40

作为一个后端工程师，想必在职业生涯中都写过一些不好维护的代码。本文是我学习《代码之丑》的学习笔记，今天第二天，品品重复代码和长函数方法的味道。

上一篇：一天一点代码坏味道（1）

1 重复代码

CVS=Ctrl C + Ctrl V + Ctrl S，没错，这就是我们每天在干的事情。

CVS一时爽，重复代码少不了。

对于重复代码最直接的建议就是DRY原则，即Don't Repeat Yourself，换成人话就是提取公共方法，然后在需要的地方，调用这个方法代替CVS。

代码结构重复

坏味道代码：

public void SendBook()
{
    try
    {
        _service.SendBook();
    }
    catch (SendFailureException ex)
    {
        _notification.Send(ex);
        throw ex;
    }
}

public void SendChapter()
{
    try
    {
        _service.SendChapter();
    }
    catch (SendFailureException ex)
    {
        _notification.Send(ex);
        throw ex;
    }
}

public void StartTranslation()
{
    try
    {
        _service.StartTranslation();
    }
    catch (SendFailureException ex)
    {
        _notification.Send(ex);
        throw ex;
    }
}

有经验的童鞋应该一眼就发现了其中的变化部分与不变的部分，不变的部分就是try-catch的结构及catch中的逻辑，而变化的则是try里面的调用service的业务逻辑。

那么，针对重复代码，最有效的就是提取方法（Extract Method）重构。

针对上面这个场景，可以提取出一个通用的方法：

public void ExecuteTask(Action action)
{
    try
    {
        action();
    }
    catch (SendFailureException ex)
    {
        _notification.Send(ex);
        throw ex;
    }
}

然后，哼哼，重构那几个Send方法，通过复用公共方法减少重复代码。这里使用了C#的Lambda表达式来声明Action委托。

对Action委托不熟悉的童鞋可以读一读微软的官方文档：https://docs.microsoft.com/zh-cn/dotnet/api/system.action

public void SendBook()
{
    ExecuteTask(() => _service.SendBook());
}

public void SendChapter()
{
    ExecuteTask(() => _service.SendChapter());
}

public void StartTranslation()
{
    ExecuteTask(() => _service.StartTranslation());
}

由此，我们可以看出，例子中的重构也并不复杂，关键还是在于：是否能发现结构上的重复。换句话说，即我们是否有足够的嗅觉发现代码中的坏味道。

选择重复

实际应用中，我们只要看到了if语句的出现，而if和else的代码块长得又比较相像，那么多半就是一个坏味道无疑了。

坏味道代码：

if (_user.IsEditor())
{
   _service.EditChapter(chapterId, title, content, true);
}
else
{
   _service.EditChapter(chapterId, title, content, false);
}

可以发现，这个if和else里面的两段代码几乎一致！但是，它真的是你有可能会写出来的代码，因为它没有错，只是因为你在写的时候只想到了if之后要做什么，而没有考虑这个if到底要干什么。

于是，重构一番：

bool approved = _user.IsEditor();
_service.EditChapter(chapterId, title, content, approved);

嗯，好看多了，满意得去接了杯开水。

当然，如果追求更远一点，那么可能会发现approved这个变量可能是一个变化点，未来如果判断的条件增加了或变化了呢，至少主方法这里不用变化。

bool approved = IsApproved(_user);
_service.EditChapter(chapterId, title, content, approved);

private bool IsApproved(User user)
{
    return user.IsEditor();
}

综述，对于重复代码，我们要想到DRY原则，关键点就是能够发现这些重复的代码，找到变化的和不变的部分，提取新方法，复用它！

不要重复自己，不要CVS！

2 长函数方法

我们每个程序员在心中对于一个方法最多应该多少行应该都不一样，有人认为20行，也有人认为100行。

在Bob大叔的《代码整洁之道》中提到，一个函数的最佳阅读体验是保持在20行以内。郑晔老师（《软件设计之美》《代码之丑》专栏作者）对自己的要求是表达能力强的动态语言如Python/Ruby，1行代码，而表达能力弱的静态语言如Java，则是10行代码。熊杰（《重构》译者）老师对Java的代码行数要求则是7行。由此可见，越是厉害资深的程序员，对函数方法行数的要求越短小。

当然，我们可以在做Code Review的时候抽查团队成员的函数方法的行数是否满足了团队所要求的最高数值，这个数值一定是和你团队成员的平均能力值相匹配的。

那么，长函数方法到底怎么产生的呢？

一般来说，有以下几个原因：

（1）以性能为由

虽然是个站不住的理由，但是的确有人这么说。但是，性能优化不应该是写代码的第一考量要素。

（2）平铺直叙

这是一个常见的理由，因为我们稍不注意就喜欢将自己想到的一点点罗列出来，这显然是写代码之前的设计工作不充分造成的。

这里我就不贴示例代码了，我相信大家只要维护过一个老系统，应该或多或少都有遇到。

之前我的团队里面，就有很多人都是这样把一个方法写到了100行+，简直不忍直视。然后我随机抽查Review的时候看到了，就让他们抽空改了，越早发现，越早重构会比较好，拖到后面上线了修改成本就更高了。

（3）一次加一点

这也是一个常见的理由，大家其实都比较委屈，毕竟自己也没做太多，只是加了一点点而已。

比如下面这个判断条件，经历过多次的需求之后，它就变成了这个样子：

if (code == 400 || code == 401 || code == 402 || ...
  || code == 500 || ...
  || ...
  || code == 10000 || ...)
{
  // 做一些错误处理
}

嗯，每个人都没错，只是结果很糟糕。

综述，长函数方法简直令人深恶痛绝，没有程序员愿意去阅读长函数方法，但自己又在不经意间写出了长函数方法。

自己写的长函数，那就让别人慢慢读去吧！

3 小结

本文总结了两类坏味道，一是重复代码，二是长函数方法。对于重复代码，我们要做的就是不要重复，争取复用。而对于长函数方法，我们则要控制行数规模，而且越低越好。

最后，感谢郑晔老师的这门《代码之丑》课程，让我受益匪浅！我也诚心把它推荐给关注Edison的各位童鞋！

参考资料

郑晔，《代码之丑》（推荐订阅学习）

Martin Flower著，熊杰译，《重构：改善既有代码的设计》（推荐至少学习第三章）

码农公寓