如何重構"箭頭型"代碼
本文主要起因是，一次在微博上和朋友關於嵌套好幾層的if-else語句的代碼重構的討論（微博原文），在微博上大家有各式各樣的問題和想法。按道理來說這些都是編程的基本功，似乎不太值得寫一篇文章，不過我覺得很多東西可以從一個簡單的東西出發，到達本質，所以，我覺得有必要在這裏寫一篇的文章。不一定全對，只希望得到更多的討論，因為有了更深入的討論才能進步。
文章有點長，我在文章最後會給出相關的思考和總結陳詞，你可以跳到結尾。
所謂箭頭型代碼，基本上來說就是下面這個圖片所示的情況。
那麽，這樣“箭頭型”的代碼有什麽問題呢？看上去也挺好看的，有對稱美。但是……
關於箭頭型代碼的問題有如下幾個：
1）我的顯示器不夠寬，箭頭型代碼縮進太狠了，需要我來回拉水平滾動條，這讓我在讀代碼的時候，相當的不舒服。
2）除了寬度外還有長度，有的代碼的if-else裏的if-else裏的if-else的代碼太多，讀到中間你都不知道中間的代碼是經過了什麽樣的層層檢查才來到這裏的。
總而言之，“箭頭型代碼”如果嵌套太多，代碼太長的話，會相當容易讓維護代碼的人（包括自己）迷失在代碼中，因為看到最內層的代碼時，你已經不知道前面的那一層一層的條件判斷是什麽樣的，代碼是怎麽運行到這裏的，所以，箭頭型代碼是非常難以維護和Debug的。
微博上的案例 與 Guard Clauses
OK，我們先來看一下微博上的那個示例，代碼量如果再大一點，嵌套再多一點，你很容易會在條件中迷失掉（下面這個示例只是那個“大箭頭”下的一個小箭頭）
FOREACH(Ptr<WfExpression>, argument, node->arguments) {
int index = manager->expressionResolvings.Keys().IndexOf(argument.Obj());
if (index != -1) {
auto type = manager->expressionResolvings.Values()[index].type;
if (! types.Contains(type.Obj())) {
types.Add(type.Obj());
上面這段代碼，可以把條件反過來寫，然後就可以把箭頭型的代碼解掉了，重構的代碼如下所示：
FOREACH(Ptr<WfExpression>, argument, node->arguments) {
int index = manager->expressionResolvings.Keys().IndexOf(argument.Obj());
if (index == -1) continue;
auto type = manager->expressionResolvings.Values()[index].type;
if ( types.Contains(type.Obj())) continue;
types.Add(type.Obj());
auto group = type->GetTypeDescriptor()->GetMethodGroupByName(L"CastResult", true);
if ( ! group ) continue;
int count = group->GetMethodCount();
for (int i = 0; i < count; i++) { auto method = group->GetMethod(i);
if (! method->IsStatic()) continue;
if ( method->GetParameterCount() == 1 &&
method->GetParameter(0)->GetType()->GetTypeDescriptor() == description::GetTypeDescriptor<DescriptableObject>() &&
method->GetReturn()->GetTypeDescriptor() != description::GetTypeDescriptor<void>() ) {
symbol->typeInfo = CopyTypeInfo(method->GetReturn());
break;
}
}
}
這種代碼的重構方式叫 Guard Clauses
- Martin Fowler 的 Refactoring 的網站上有相應的說明《Replace Nested Conditional with Guard Clauses》。
- Coding Horror 上也有一篇文章講了這種重構的方式 —— 《Flattening Arrow Code》
- StackOverflow 上也有相關的問題說了這種方式 —— 《Refactor nested IF statement for clarity》
這裏的思路其實就是，讓出錯的代碼先返回，前面把所有的錯誤判斷全判斷掉，然後就剩下的就是正常的代碼了。
抽取成函數
微博上有些人說，continue 語句破壞了閱讀代碼的通暢，我覺得他們一定沒有好好讀這裏面的代碼，其實，我們可以看到，所有的 if 語句都是在判斷是否出錯的情況，所以，在維護代碼的時候，你可以完全不理會這些 if 語句，因為都是出錯處理的，而剩下的代碼都是正常的功能代碼，反而更容易閱讀了。當然，一定有不是上面代碼裏的這種情況，那麽，不用continue ，我們還能不能重構呢？
當然可以，抽成函數：
bool CopyMethodTypeInfo(auto &method, auto &group, auto &symbol)
{
if (! method->IsStatic()) {
return true;
}
if ( method->GetParameterCount() == 1 &&
method->GetParameter(0)->GetType()->GetTypeDescriptor() == description::GetTypeDescriptor<DescriptableObject>() &&
method->GetReturn()->GetTypeDescriptor() != description::GetTypeDescriptor<void>() ) {
symbol->typeInfo = CopyTypeInfo(method->GetReturn());
return false;
}
return true;
}
void ExpressionResolvings(auto &manager, auto &argument, auto &symbol)
{
int index = manager->expressionResolvings.Keys().IndexOf(argument.Obj());
if (index == -1) return;
auto type = manager->expressionResolvings.Values()[index].type;
if ( types.Contains(type.Obj())) return;
types.Add(type.Obj());
auto group = type->GetTypeDescriptor()->GetMethodGroupByName(L"CastResult", true);
if ( ! group ) return;
int count = group->GetMethodCount();
for (int i = 0; i < count; i++) { auto method = group->GetMethod(i);
if ( ! CopyMethodTypeInfo(method, group, symbol) ) break;
}
}
...
...
FOREACH(Ptr<WfExpression>, argument, node->arguments) {
ExpressionResolvings(manager, arguments, symbol)
}
你發出現，抽成函數後，代碼比之前變得更容易讀和更容易維護了。不是嗎？
有人說：“如果代碼不共享，就不要抽取成函數！”，持有這個觀點的人太死讀書了。函數是代碼的封裝或是抽象，並不一定用來作代碼共享使用，函數用於 屏蔽細節，讓其它代碼耦合於接口而不是細節實現，這會讓我們的代碼更為簡單，簡單的東西都能讓人易讀也易維護。這才是函數的作用。
嵌套的 if 外的代碼
微博上還有人問，原來的代碼如果在各個 if 語句後還有要執行的代碼，那麽應該如何重構。比如下面這樣的代碼。
原版
for(....) {
do_before_cond1()
if (cond1) {
do_before_cond2();
if (cond2) {
do_before_cond3();
if (cond3) {
do_something();
}
do_after_cond3();
}
do_after_cond2();
}
do_after_cond1();
}
上面這段代碼中的那些 do_after_condX() 是無論條件成功與否都要執行的。所以，我們拉平後的代碼如下所示：
重構第一版
for(....) {
do_before_cond1();
if ( !cond1 ) {
do_after_cond1();
continue
}
do_after_cond1();
do_before_cond2();
if ( !cond2 ) {
do_after_cond2();
continue;
}
do_after_cond2();
do_before_cond3();
if ( !cond3 ) {
do_after_cond3();
continue;
}
do_after_cond3();
do_something();
}
你會發現，上面的 do_after_condX 出現了兩份。如果 if 語句塊中的代碼改變了某些do_after_condX依賴的狀態，那麽這是最終版本。
但是，如果它們之前沒有依賴關系的話，根據 DRY 原則，我們就可以只保留一份，那麽直接掉到 if 條件前就好了，如下所示：
重構第二版
for(....) {
do_before_cond1();
do_after_cond1();
if ( !cond1 ) continue;
do_before_cond2();
do_after_cond2();
if ( !cond2 ) continue;
do_before_cond3();
do_after_cond3();
if ( !cond3 ) continue;
do_something();
}
此時，你會說，我靠，居然，改變了執行的順序，把條件放到 do_after_condX() 後面去了。這會不會有問題啊？
其實，你再分析一下之前的代碼，你會發現，本來，cond1 是判斷 do_before_cond1() 是否出錯的，如果有成功了，才會往下執行。而 do_after_cond1() 是無論如何都要執行的。從邏輯上來說，do_after_cond1()其實和do_before_cond1()的執行結果無關，而 cond1 卻和是否去執行 do_before_cond2() 相關了。如果我把斷行變成下面這樣，反而代碼邏輯更清楚了。
重構第三版
for(....) {
do_before_cond1();
do_after_cond1();
if ( !cond1 ) continue; // <-- cond1 成了是否做第二個語句塊的條件
do_before_cond2();
do_after_cond2();
if ( !cond2 ) continue; // <-- cond2 成了是否做第三個語句塊的條件
do_before_cond3();
do_after_cond3();
if ( !cond3 ) continue; //<-- cond3 成了是否做第四個語句塊的條件
do_something();
}
於是乎，在未來維護代碼的時候，維護人一眼看上去就明白，代碼在什麽時候會執行到哪裏。 這個時候，你會發現，把這些語句塊抽成函數，代碼會幹凈的更多，再重構一版：
重構第四版
bool do_func3() {
do_before_cond2();
do_after_cond2();
return cond3;
}
bool do_func2() {
do_before_cond2();
do_after_cond2();
return cond2;
}
bool do_func1() {
do_before_cond1();
do_after_cond1();
return cond1;
}
// for-loop 你可以重構成這樣
for (...) {
bool cond = do_func1();
if (cond) cond = do_func2();
if (cond) cond = do_func3();
if (cond) do_something();
}
// for-loop 也可以重構成這樣
for (...) {
if ( ! do_func1() ) continue;
if ( ! do_func2() ) continue;
if ( ! do_func3() ) continue;
do_something();
}
上面，我給出了兩個版本的for-loop，你喜歡哪個？我喜歡第二個。這個時候，因為for-loop裏的代碼非常簡單，就算你不喜歡 continue ，這樣的代碼閱讀成本已經很低了。
狀態檢查嵌套
接下來，我們再來看另一個示例。下面的代碼的偽造了一個場景——把兩個人拉到一個一對一的聊天室中，因為要檢查雙方的狀態，所以，代碼可能會寫成了“箭頭型”。
int ConnectPeer2Peer(Conn *pA, Conn* pB, Manager *manager)
{
if ( pA->isConnected() ) {
manager->Prepare(pA);
if ( pB->isConnected() ) {
manager->Prepare(pB);
if ( manager->ConnectTogther(pA, pB) ) {
pA->Write("connected");
pB->Write("connected");
return S_OK;
}else{
return S_ERROR;
}
}else {
pA->Write("Peer is not Ready, waiting...");
return S_RETRY;
}
}else{
if ( pB->isConnected() ) {
manager->Prepare();
pB->Write("Peer is not Ready, waiting...");
return S_RETRY;
}else{
pA->Close();
pB->Close();
return S_ERROR;
}
}
//Shouldn‘t be here!
return S_ERROR;
}
重構上面的代碼，我們可以先分析一下上面的代碼，說明了，上面的代碼就是對 PeerA 和 PeerB 的兩個狀態 “連上”， “未連上” 做組合 “狀態” （註：實際中的狀態應該比這個還要復雜，可能還會有“斷開”、“錯誤”……等等狀態）， 於是，我們可以把代碼寫成下面這樣，合並上面的嵌套條件，對於每一種組合都做出判斷。這樣一來，邏輯就會非常的幹凈和清楚。
int ConnectPeer2Peer(Conn *pA, Conn* pB, Manager *manager)
{
if ( pA->isConnected() ) {
manager->Prepare(pA);
}
if ( pB->isConnected() ) {
manager->Prepare(pB);
}
// pA = YES && pB = NO
if (pA->isConnected() && ! pB->isConnected() ) {
pA->Write("Peer is not Ready, waiting");
return S_RETRY;
// pA = NO && pB = YES
}else if ( !pA->isConnected() && pB->isConnected() ) {
pB->Write("Peer is not Ready, waiting");
return S_RETRY;
// pA = YES && pB = YES
}else if (pA->isConnected() && pB->isConnected() ) {
if ( ! manager->ConnectTogther(pA, pB) ) {
return S_ERROR;
}
pA->Write("connected");
pB->Write("connected");
return S_OK;
}
// pA = NO, pB = NO
pA->Close();
pB->Close();
return S_ERROR;
}
延伸思考
對於 if-else 語句來說，一般來說，就是檢查兩件事：錯誤 和 狀態。
檢查錯誤
對於檢查錯誤來說，使用 Guard Clauses 會是一種標準解，但我們還需要註意下面幾件事：
1）當然，出現錯誤的時候，還會出現需要釋放資源的情況。你可以使用 goto fail; 這樣的方式，但是最優雅的方式應該是C++面向對象式的 RAII 方式。
2）以錯誤碼返回是一種比較簡單的方式，這種方式有很一些問題，比如，如果錯誤碼太多，判斷出錯的代碼會非常復雜，另外，正常的代碼和錯誤的代碼會混在一起，影響可讀性。所以，在更為高組的語言中，使用 try-catch 異常捕捉的方式，會讓代碼更為易讀一些。
檢查狀態
對於檢查狀態來說，實際中一定有更為復雜的情況，比如下面幾種情況：
1）像TCP協議中的兩端的狀態變化。
2）像shell各個命令的命令選項的各種組合。
3）像遊戲中的狀態變化（一棵非常復雜的狀態樹）。
4）像語法分析那樣的狀態變化。
對於這些復雜的狀態變化，其本上來說，你需要先定義一個狀態機，或是一個子狀態的組合狀態的查詢表，或是一個狀態查詢分析樹。
寫代碼時，代碼的運行中的控制狀態或業務狀態是會讓你的代碼流程變得混亂的一個重要原因，重構“箭頭型”代碼的一個很重要的工作就是重新梳理和描述這些狀態的變遷關系。
總結
好了，下面總結一下，把“箭頭型”代碼重構掉的幾個手段如下：
1）使用 Guard Clauses 。 盡可能的讓出錯的先返回， 這樣後面就會得到幹凈的代碼。
2）把條件中的語句塊抽取成函數。 有人說：“如果代碼不共享，就不要抽取成函數！”，持有這個觀點的人太死讀書了。函數是代碼的封裝或是抽象，並不一定用來作代碼共享使用，函數用於屏蔽細節，讓其它代碼耦合於接口而不是細節實現，這會讓我們的代碼更為簡單，簡單的東西都能讓人易讀也易維護，寫出讓人易讀易維護的代碼才是重構代碼的初衷！
3）對於出錯處理，使用try-catch異常處理和RAII機制。返回碼的出錯處理有很多問題，比如：A) 返回碼可以被忽略，B) 出錯處理的代碼和正常處理的代碼混在一起，C) 造成函數接口汙染，比如像atoi()這種錯誤碼和返回值共用的糟糕的函數。
4）對於多個狀態的判斷和組合，如果復雜了，可以使用“組合狀態表”，或是狀態機加Observer的狀態訂閱的設計模式。這樣的代碼即解了耦，也幹凈簡單，同樣有很強的擴展性。
5） 重構“箭頭型”代碼其實是在幫你重新梳理所有的代碼和邏輯，這個過程非常值得為之付出。重新整思路去想盡一切辦法簡化代碼的過程本身就可以讓人成長。
如何重構"箭頭型"代碼