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

  • 2010-11-22
  • 本文字数:2407 字

    阅读完需:约 8 分钟

这是一个长长的判断条件:


if (strcmp(type, “DropGroup") == 0
   || strcmp(type, "CancelUserGroup") == 0
   || strcmp(type, "QFUserGroup") == 0
   || strcmp(type, "CancelQFUserGroup") == 0
   || strcmp(type, "QZUserGroup") == 0
   || strcmp(type, "CancelQZUserGroup") == 0
   || strcmp(type, "SQUserGroup") == 0
   || strcmp(type, "CancelSQUserGroup") == 0
   || strcmp(type, “UseGroup") == 0
   || strcmp(type, "CancelGroup") == 0)

之所以注意到它,因为最后两个条件是在最新修改里面加入的,换句话说,这不是一次写就的代码。单就这一次而言,只改了两行,这是可以接受的。但这是遗留代码,每次可能只改了一两行,通常我们会不只一次踏入这片土地。经年累月,代码成了这个样子。

就我接触过的代码而言,这并不是最长的判断条件。这种代码极大的开拓了我的视野,现在的我,即便面前是一屏无法容纳的条件,也可以坦然面对了,虽然显示器越来越大。

其实,如果这个判断条件是这个函数里仅有的东西,我也是可以接受的。遗憾的是,大多数情况下,这只不过是一个更大函数中的一小段而已。为了让这段代码可以接受一些,我们不妨稍做封装:


 bool shouldExecute(const char* type) {
   return (strcmp(type, “DropGroup") == 0
   || strcmp(type, "CancelUserGroup") == 0
   || strcmp(type, "QFUserGroup") == 0
   || strcmp(type, "CancelQFUserGroup") == 0
   || strcmp(type, "QZUserGroup") == 0
   || strcmp(type, "CancelQZUserGroup") == 0
   || strcmp(type, "SQUserGroup") == 0
   || strcmp(type, "CancelSQUserGroup") == 0
   || strcmp(type, “UseGroup") == 0
   || strcmp(type, "CancelGroup") == 0);
 }
 if (shouldExecute(type)) {
   ...
 }

现在,虽然条件依然还是很多,但比起原来庞大的函数,至少它已经被控制在一个相对较小的函数里了。更重要的是,通过函数名,我们终于有机会告诉世人这段代码判断的是什么了。

虽然提取函数把这段代码混乱的条件分离开来,它还是可以继续改进的。比如,我们把判断的条件进一步提取:


bool shouldExecute(const char* type) {
 static const char* execute_type[] = {
   "DropGroup",
   "CancelUserGroup",
   "QFUserGroup",
   "CancelQFUserGroup",
   "QZUserGroup",
   "CancelQZUserGroup",
   "SQUserGroup",
   "CancelSQUserGroup",
   "UseGroup",
   "CancelGroup"
 };
 int size = ARRAY_SIZE(execute_type);
 for (int i = 0; i < size; i++) {
   if (strcmp(type, execute_type[i]) == 0) {
     return true;
   }
 }
 return false;
}

这样的话,如果以后要加一个新的 type,只要在数组中增加一个新的元素即可。这段代码还可以进一步封装,把这个 type 列表变成声明式,进一步提高代码的可读性。

简单的理解声明式的风格,就是描述做什么,而不是怎么做。一个声明式编程的例子是 Rails 里面的数据关联,为人熟知的 has_many 和 belongs_to。通过这个声明,模型类就会具备一些数据关联的能力。具体到实际开发里,声明式编程需要有两个部分:一方面是一些基础的框架性代码,另一方面是应用层面如何使用。通常,框架代码不像应用层面代码那么好理解,但有了这个基础,应用代码就会变得简单许多。针对上面那段代码,按照这种风格,我改造了代码,下面是框架部分的代码:


#define BEGIN_STR_PREDICATE(predicate_name) \
bool predicate_name(const char* field) { \
 static const char* predicate_true_fields[] = {
#define STR_PREDICATE_ITEM(item) #item ,
#define END_STR_PREDICATE \
 };\
 \
 int size = ARRAY_SIZE(predicate_true_fields);\
 for (int i = 0; i < size; i++) { \
   if (strcmp(field, predicate_true_fields[i]) == 0) {\
       return true;\
   }\
 }\
\
 return false;\
} 

这里用到了 C/C++ 常见的宏技巧,为的就是让应用层面的代码写起来更像声明。稍微对比一下,就会发现,实际上二者几乎是一样的。有了框架,就该应用了:


BEGIN_STR_PREDICATE(shouldExecute)
 STR_PREDICATE_ITEM(DropGroup )
 STR_PREDICATE_ITEM(CancelUserGroup )
 STR_PREDICATE_ITEM(QFUserGroup )
 STR_PREDICATE_ITEM(CancelQFUserGroup )
 STR_PREDICATE_ITEM(QZUserGroup )
 STR_PREDICATE_ITEM(CancelQZUserGroup )
 STR_PREDICATE_ITEM(SQUserGroup )
 STR_PREDICATE_ITEM(CancelSQUserGroup )
 STR_PREDICATE_ITEM(UseGroup )
 STR_PREDICATE_ITEM(CancelGroup )
END_STR_PREDICATE 

shouldExecute 就此重现出来了。不过,这段代码已经不再像一个函数,而更像一段声明,而这,恰恰就是我们的目标。有了这个基础,实现一个新的函数,不过是做一个新的声明而已。

使用这个新函数的方法依然如故:


if (shouldExecute(type)) {
   ...
 }

虽然应用代码变得简单了,但写出框架的结构是需要一定基础的。它不像应用代码那样来得平铺直叙,但其实也没那么难,只不过很多人从没有考虑把代码写成这样。只要换个角度去思考,多多练习,也就可以驾轻就熟了。

发现这种代码很容易,只要看到在长长的判断条件,就是它了。要限制这种代码的存在,只要以设定一个简单的规则:

  • 判断条件里面不允许多个条件的组合

在实际的应用中,我们会把“3”定义为“多”,也就是如果有两个条件的组合,可以接受,如果是三个,还是改吧!

虽然通过不断调整,这段代码已经不同于之前,但它依然不是我们心目中的理想代码。出现这种代码,往往意味背后有更严重的设计问题。不过,它并不是这里讨论的内容,这里的讨论就到此为止吧!

作者简介:

郑晔,ThoughtWorks 公司咨询师,拥有多年企业级软件开发经验,热衷于探索各种程序设计语言在真实软件开发中所能发挥的威力,致力于探寻合理的软件开发方式,加入 ThoughtWorks 公司后,投入到敏捷开发方法的实践之中,为其他公司提供敏捷开发方法方面的咨询服务。他的 blog 是梦想风暴

查看原文:代码之丑(二)代码之丑(二)(续)