CATEGORY / Development

代码之初,性本丑?(上)

Permanent Link: http://wutiam.net/notes/143

我是一个很在意代码质量的人,因为我相信软件开发,细节决定成败(追求细节和拥有宏观视野本身并不矛盾,而且这是另一个话题了)。虽然我并不赞成将团队甚至公司中的每个人的编码风格都硬性规范到某种统一格式,那样不仅会扼杀程序员们的创造力,而且从成本收益来看太不划算了。但是,这并不意味着代码就可以随心所欲地写,所谓“规范”不该是“法律”上的,但一定是“道德”上的:你现在写的代码,要对半年后的你负责,更要对团队里其他同事们负责,甚至可能还有你的客户。而这种责任,不仅仅是代码的功能完整度,更包括可读健壮效率

这几个月有幸参与了我们游戏提升稳定性和优化性能的工作,review了很多代码,这期间时长被各种“富有创造性的”代码雷得外焦里嫩的。碰巧正好看到了《代码之丑》系列文章,感触颇深,有些观点也有些自己的看法,记录下来,与大家探讨。

代码之丑——开篇

功能完整的代码,仅仅指的是“能按流程跑起来、不会崩、可以得到正确结果”的代码,这其中的代码质量依然可谓千差万别。在大多数情况下,这些代码可以定义其美丑。不过该文中举的例子可能不够典型,不够丑陋,结果被大家揪住了小尾巴。的确,两种写法都有各自大量的拥趸,也都有各自的理由:比如用 if/else 分别 return 的同学更在意其可 debug 性,而且如果今后要在 if 或 else 的 {} 里加入更多的处理(不仅仅是 return)也很方便;而采用表达式直接 return 的同学更喜欢代码的简洁性。这种时候,你很难让人信服地把这类代码归为丑陋,这只是个人口味不同。

但另一种类似的情况就截然不同了,比如这个函数:

inline bool Foo(void)
{
    // 整个函数代码很少,几乎没什么额外功能
    ...
 
    if(db.HasNext()) {
        return true;
    } else {
       return false;
}

甚至判断条件简单到只有“db != NULL”之类的语义本身就是纯条件判断的,那这时傻乎乎的用 if/else 就可能贻笑大方了,有人会写出 if (true) return true; else return false; 的代码么?

代码之丑(一)——让判断条件做真正的选择

这篇中提到的问题其实非常常见,因为正常人一般都是先写完了某个函数的调用,然后改吧改吧才发现在不同情况下需要使用不同的参数调这个函数,或者当时写代码时只是想验证这条路是否可行,没太在意避免重复的代码,验证通过后又懒的再去修改已经稳定了的代码。该文中的例子算是客气的,我们有不少代码是在多层 if/else 的每个 {} 中都复制了代码,函数调用本身就很长,那么多参数,有差别的只有几个字母,如果哪天这个函数的输入输出改了,想必维护这段代码的同学很难不改出 bug。

程序中最忌讳的陋习之一就是复制代码,这对软件的影响不仅仅是增大程序体积,真正严重的是每当需要修改这段代码,所有它的副本都得记得去改,如果漏掉了一个,哼哼,一个小时内找到问题就算不错了。可又有多少程序员在编码的时候会提醒自己这个问题呢?我想恐怕不超过10%,至少真的能要求自己做到的不会超过这个比例。聪明的懒人,是努力让自己日后更轻松,而不是偷一时之快让自己逐渐进入越忙越乱越乱越忙的恶性循环。

不过,话又要说回来,任何的重构都要有个度,也要有个合适的环境才行。重构的前提是要保持语义清晰,或者是为了语义更清晰而重构,而不能为了重构而重构。对于该文的这个最最简单的例子来说,下面 Shichao Liu 的补充给得很强力:

对于仅仅参数不同分支, 也要从分支的业务意义上加以区分:
1.是恰巧逻辑相同,实则业务意义上完全独立的分支。
2.或是业务意义上就应该有完全相同的逻辑操作,而仅是数据有差异。

在需求发生演变时会有所不同。

举个例子,网游, 一次攻击可能是“普通伤害”或者“致命一击”。可能在第一个版本中仅仅是伤害数值有所差异, 而在第二个版本中需要对致命一击后追击一些额外的操作(统计次数,累积点数等等策划能想得到的点子)。

代码之丑(二)——长长的条件

这篇我的认同度就比较低了,的确,该文所述的方法是各大软件公司惯用的技巧(MFC 里就有好多),重构出来的代码看似也非常清晰。但,对于第一次阅读或者修改这个代码的人,第一反应估计就是“STR_PREDICATE_ITEM 这个宏都干了啥?靠谱么?后面要不要加分号?BEGIN 和 END 之间加上 {} 会有问题么?太可怕了,又得记住一堆宏定义和用法,而且其实我总是记不起来要去用它们……”这还不是最糟糕的。你不能要求所有的同事都有这种归纳重构的能力,而且就算大家都是重构大拿,在一个项目中把各自写的所有复杂判断条件都这么折腾一趟,要么每个人得给每个判断单独写一套宏(很快就不会有人遵守了,今天还有五个功能等着交付呢),要么有神人来写了套相当泛型的通用宏,哇哦,原本为了可读性而重构出来的代码变得完全没法维护了。最严酷的是,如果条件并不是简单而统一的判断,比如:

if (IsOperator()
    || (IsOtherPlayer() && GetID() < 100)
    || (IsNpc() && (!IsMonster() || strResName != "NNN"))
    || strResName == "RRR"
    || strResName.Contain("XY")
    ...)

怎么办?更复杂的多层嵌套呢?应该说出现这类判断的代码,背后的设计一定是有问题的,但如果我们没法解决设计的缺陷,只能努力找一个平衡点,别累死写代码的人,也别搞懵维护代码的人,更别为了追求代码看起来很上流而让所有人都感到反感。

No Comments / Trackbacks / Pingbacks

Leave a Reply

:) :wink: 8-O :lol: :-D 8) :-| :mrgreen: :oops: :-o :-? :( :twisted: :cry: more »