上次有人來要求我寫一篇文章談談什么代碼才是好代碼,是誰我已經忘記了,好像是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';'); // 1
if(!p) throw ArgumentException(L"Illegal config string"); // 2
DoName(wstring(config, p)); // 3
auto q = wcschr(p + 1, L';'); // 4
if(!q) throw ArgumentException(L"Illegal config string"); // 5
DoParent(wstring(p + 1, q); // 6
auto r = wcschr(q + 1, L';'); // 7
if(r) throw ArgumentException(L"Illegal config string"); // 8
DoDescription(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了,你的代碼就會不同程度的出現各種腐爛或者破窗,上面那三條其實只是我能想到的比較常見的表現形式。這件事情也告訴我們,當高手告訴你什么什么不能做的時候,得想一想背后的原因,不然跟封建迷信有什么區別。
posted on 2014-07-15 06:44
陳梓瀚(vczh) 閱讀(15795)
評論(9) 編輯 收藏 引用 所屬分類:
啟示