转：靠谱的代码和DRY

http://www.cppblog.com/vczh/archive/2014/07/15/207658.html

 

靠谱的代码和DRY

上次有人来要求我写一篇文章谈谈什么代码才是好代码，是谁我已经忘记了，好像是AutoHotkey还是啥的专栏的作者。撇开那些奇怪的条款不谈，靠谱的 代码有一个共同的特点，就是DRY。DRY就是Don't Repeat Yourself，其实已经被人谈了好多年了，但是几乎所有人都会忘记。

什么是DRY（Don't Repeat Yourself） 

DRY 并不是指你不能复制代码这么简单的。不能repeat的其实是信息，不是代码。要分析一段代码里面的什么东西时信息，就跟给物理题做受力分析一样，想每次 都做对其实不太容易。但是一份代码总是要不断的修补的，所以在这之前大家要先做好TDD，也就是Test Driven Development。这里我对自己的要求是覆盖率要高达95%，不管用什么手段，总之95%的代码的输出都要受到检验。当有了足够多的测试做后盾的时 候，不管你以后发生了什么，譬如说你发现你Repeat了什么东西要改，你才能放心大胆的去改。而且从长远的角度来看，做好TDD可以将开发出相同质量的代码的时间缩短到30%左右（这是我自己的经验值） 。

什么是信息

信息这个词不太好用语言下定义，不过我可以举个例子。譬如说你要把一个配置文件里面的字符串按照分隔符分解成几个字符串，你大概就会写出这样的代码：

// name;parent;description
void ReadConfig(const wchar_t* config)
{auto p = wcschr(config, L';');                            // 1if(!p) throw ArgumentException(L"Illegal config string"); // 2DoName(wstring(config, p));                               // 3auto q = wcschr(p + 1, L';');                             // 4if(!q) throw ArgumentException(L"Illegal config string"); // 5DoParent(wstring(p + 1, q);                               // 6auto r = wcschr(q + 1, L';');                             // 7if(r) throw ArgumentException(L"Illegal config string");  // 8DoDescription(q + 1);                                     // 9
}

这段短短的代码重复了多少信息？

• 分隔符用的是分号（1、4、7）
• 第二/三个片段的第一个字符位于第一/二个分号的后面（4、6、7、9）
• 格式检查（2、5、8）
• 异常内容（2、5、8）

除了DRY以外还有一个问题，就是处理description的方法跟name和parent不一样，因为他后面再也没有分号了。

那这段代码要怎么改呢？有些人可能会想到，那把重复的代码抽取出一个函数就好了：

wstring Parse(const wchar_t& config, bool end)
{auto next = wcschr(config, L';');ArgumentException up(L"Illegal config string");if (next){if (end) throw up;wstring result(config, next);config = next + 1;return result;}else{if (!end) throw up;wstring result(config);config += result.size();return result;}
}// name;parent;description
void ReadConfig(const wchar_t* config)
{DoName(Parse(config, false));DoParent(Parse(config, false));DoDescription(Parse(config, true));
}

是不是看起来还很别扭，好像把代码修改了之后只把事情搞得更乱了，而且就算config对了我们也会创建那个up变量，就仅仅是为了不 重复代码。而且这份代码还散发出了一些不好的味道，因为对于Name、Parent和Description的处理方法还是不能统一，Parse里面针对 end变量的处理看起来也是很重复，但实际上这是无法在这样设计的前提下消除的。所以这个代码也是不好的，充其量只是比第一份代码强一点点。

实 际上，代码之所以要写的好，之所以不能repeat东西，是因为产品狗总是要改需求，不改代码你就要死，改代码你就要加班，所以为了减少修改代码的痛苦， 我们不能repeat任何信息。举个例子，有一天产品狗说，要把分隔符从分号改成空格！一下子就要改两个地方了。description后面要加tag！ 这样你处理description的方法又要改了因为他是以空格结尾不是0结尾。

因此针对这个片段，我们需要把它改成这样：

vector<wstring> SplitString(const wchar_t* config, wchar_t delimiter)
{vector<wstring> fragments;while(auto next = wcschr(config, delimiter)){fragments.push_back(wstring(config, next));config = next + 1;}fragments.push_back(wstring(config));return fragments; // C++11就是好！
}void ReadConfig(const wchar_t* config)
{auto fragments = SplitString(config, L';');if(fragments.size() != 3){throw ArgumentException(L"Illegal config string");}DoName(fragments[0]);DoParent(fragments[1]);DoDescription(fragments[2]);
}

我们可以发现，分号（L';'）在这里只出现了一次，异常内容也只出现了一次，而且处理name、parent和 description的代码也没有什么区别了，检查错误也更简单了。你在这里还给你的Library增加了一个SplitString函数，说不定在以 后什么地方就用上了，比Parse这种专门的函数要强很多倍。

大家可以发现，在这里重复的东西并不仅仅是复制了代码，而是由于你把 同一个信息散播在了代码的各个部分导致了有很多相近的代码也散播在各个地方，而且还不是那么好通过抽成函数的方法来解决。因为在这种情况下，就算你把重复 的代码抽成了Parse函数，你把函数调用了几次实际上也等于重复了信息。因此正确的方法就是把做事情的方法变一下，写成SplitString。这个 SplitString函数并不是通过把重复的代码简单的抽取成函数而做出来的。去掉重复的信息会让你的代码的结构发生本质的变化。

这个问题其实也有很多变体：

• 不能有Magic Number。L';'出现了很多遍，其实就是个Magic Number。所以我们要给他个名字，譬如说delimiter。
• 不要复制代码。这个应该不用我讲了。 
• 解耦要做成正交的。SplitString虽然不是直接冲着读config来写的，但是它反映了一个在其它地方也会遇到的常见的问题。如果用Parse的那个版本，显然只是看起来解决了问题而已，并没有给你带来任何额外的效益。

信息一旦被你repeat了，你的代码就会不同程度的出现各种腐烂或者破窗，上面那三条其实只是我能想到的比较常见的表现形式。这件事情也告诉我们，当高手告诉你什么什么不能做的时候，得想一想背后的原因，不然跟封建迷信有什么区别。