一行代码修改引起的血案

最近在做代码审查 Code Review 并顺便使用了商业的软件 Resharper (Visual Studio 的插件, 有 30 天试用)

Resharper 一个很好的功能就是能把没有用的代码变灰, 这样哪几行代码没有用就一目了然.

代码审核 C# 枚举判断成 null

像这行代码, Resharper 很清楚的提示说没有用, 因为 activeTool 是枚举类型, 和 null 判断永远为 false, 这样 if 里面的代码就永远不可能被执行, 所以标记成灰的了.

这个函数不是我写的(模块也不是我负责的), 我从 SVN blame changes 也没有发现最开始的记录, 应该是最最开始上传到SVN就有了. 可想而知, 这行代码将近有三四年没有人发现有问题. 很可怕.

由于这行代码所在的函数名称为 Initialize 我就理所当然的认为这个函数最开始初始化的时候调用一次, 并且根据上一行的英文注释 // set the default tool 我就认为应该去掉 if 直接写成

activeTool = LayoutToolType.Pointer;

结果过几天, 同事Q我说这个改动引起一个错误, 并改回去了(回退), 我就觉得纳闷, 当然如果我不自做聪明的话, 应该直接把这两行(包括上一行注释)一并删掉, 但是我认为这样的改法只是在将错就错, 后面的逻辑肯定是错误的. 这个变量的值在哪里初始化了? 也许这个不重要.

快速的修改 bug, 并不等于, 不求甚解的 fix bug. 这原先的代码很误导, 后面的逻辑又是基于这个假设(已经初始化变量), 所以就是在错误的代码上做补救. 于是我让相关的开发人员彻底查一下这个 root cause 知其然, 而且要知其所以然.

当然,这件事上我也有错, 错在, 一:太相信自己判断的能力了(所以没有测试), 二:太相信代码的质量和开发人员的水平.

GD Star Rating
loading...

本文一共 519 个汉字, 你数一下对不对.

扫描二维码，分享本文到微信朋友圈