1. 程式人生 > >如何重構多重巢狀“箭頭型”程式碼

如何重構多重巢狀“箭頭型”程式碼

本文轉載自 酷 殼 – CoolShell 陳皓。

所謂箭頭型程式碼,基本上來說就是下面這個圖片所示的情況。
這裡寫圖片描述

那麼,這樣“箭頭型”的程式碼有什麼問題呢?看上去也挺好看的,有對稱美。但是……
關於箭頭型程式碼的問題有如下幾個:

1)我的顯示器不夠寬,箭頭型程式碼縮排太狠了,需要我來回拉水平滾動條,這讓我在讀程式碼的時候,相當的不舒服。
2)除了寬度外還有長度,有的程式碼的if-else裡的if-else裡的if-else的程式碼太多,讀到中間你都不知道中間的程式碼是經過了什麼樣的層層檢查才來到這裡的。
總而言之,“箭頭型程式碼”如果巢狀太多,程式碼太長的話,會相當容易讓維護程式碼的人(包括自己)迷失在程式碼中,因為看到最內層的程式碼時,你已經不知道前面的那一層一層的條件判斷是什麼樣的,程式碼是怎麼執行到這裡的,所以,箭頭型程式碼是非常難以維護和Debug的。

案例 與 Guard Clauses

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());
            if
(auto group = type->GetTypeDescriptor()->GetMethodGroupByName(L"CastResult", true)) { int count = group->GetMethodCount(); for (int i = 0; i < count; i++) { auto method = group->GetMethod(i); if (method->IsStatic()) { if
(method->GetParameterCount() == 1 && method->GetParameter(0)->GetType()->GetTypeDescriptor() == description::GetTypeDescriptor<DescriptableObject>() && method->GetReturn()->GetTypeDescriptor() != description::GetTypeDescriptor<void>() ) { symbol->typeInfo = CopyTypeInfo(method->GetReturn()); break; } } } } } } }

上面這段程式碼,可以把條件反過來寫,然後就可以把箭頭型的程式碼解掉了,重構的程式碼如下所示:

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) 重構“箭頭型”程式碼其實是在幫你重新梳理所有的程式碼和邏輯,這個過程非常值得為之付出。重新整思路去想盡一切辦法簡化程式碼的過程本身就可以讓人成長。