如何重构"箭头型"代码

本文主要起因是，一次在微博上和朋友关于嵌套好几层的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());
            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

这里的思路其实就是，让出错的代码先返回，前面把所有的错误判断全判断掉，然后就剩下的就是正常的代码了。

抽取成函数

微博上有些人说，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） 重构“箭头型”代码其实是在帮你重新梳理所有的代码和逻辑，这个过程非常值得为之付出。重新整思路去想尽一切办法简化代码的过程本身就可以让人成长。

时间： 2024-10-11 00:07:49

如何重构"箭头型"代码的相关文章

如何正确重构你的代码

题记:刚工作那会给新来组里的同事写的技术文章,用了一些轻松的口吻,当时的效果很好.现在翻看起来,觉得当小说看还是可以的.具体内容其实参考了<重构>以及另一本也是重构的书. 说到重构,什么时候你的代码要重构呢?有这么一个测试方法:“如果你发现修改自己的代码感到很崩溃,如果你发现阅读自己的代码之前需要先查看注释,这就表明你的代码需要重构了”. 大家知道,大型软件项目的开发是一个长期的,团体协作开发的过程,对于软件中的某一个模块,某一个功能,甚至是某一段代码,都有几个甚至十几个人修改过,你可以想象你

重构摘要3_代码的坏味道

如果尿布臭了,就换掉它. 1.Duplicated Code 重复代码 Extract Method Pull Up Method Form Template Method --> Template Method 模式 Substitute Algorithm --> 函数算法替代 2.Long Method 过长的函数 "间接层"所带来的全部利益--解释能力.共享能力.选择能力--都是有小函数支持的. 真正关键在于一个好名字. 每当感觉需要以注释来说明点什么的时候,我们就

重构糟糕的代码(一)

重构不止是代码整理,它提供了一种高效且受控的代码整理技术. (一)重构原则 1.何谓重构 对软件内部结构的一种调整,目的是在不改变软件可观察行为的前提下,提高其可理解性,降低其修改成本. 另一种解释是:使用一系列重构手法,在不改变软件可观察行为的前提下,调整其结构. 2.为何重构 改进软件设计:如果没有重构,程序的设计会逐渐变质,重构很像是在整理代码,你所做的就是让所有的东西回到应处的位置上. 帮助找到bug:对代码进行重构,可以深入理解代码的作为,在搞清楚程序结构的同时,想不把bug揪出来都难

重构心法——提高代码复用率

由于现在的互联网企业业务比较繁忙,导致产品狗不停地提需求,还总是改来改去,最后留给程序猿的时间少之又少.程序猿也不是吃素的,干脆直接copy一下代码随便搞一下实现功能就行,也谈不上所谓的精心设计了.这样的确是缩短了开发的时间,但是到处都是重复代码,大大提高了软件维护的成本,为日后软件的发展带来了隐患.如果同一功能被复制粘贴了上百次,一旦这段代码需要变更时,那简直就是一种灾难,改了这边却忘了改那边.同一段代码,当你Copy第一次的时候也许还能容忍,但是第二次的时候,你应该停下来思考一下了,这是一种

数组转List-典型代码缺陷分析(三)

以上为开发过程中,部分程序猿数组转list的"笨"方法,为什么说笨呢,因为这样做代码很繁琐不简练容易出错可读性叫差,而且还比较耗时,因为我要一个一个遍历数组,然后把这个元素添加到list中(不过以上代码还有几点,最好给ArrayList<String>指定一个初始容量,注意和LinkedList的区别,以及split方法使用的效率和可能内存泄漏问题,此文不再详述,此处重点解读数组转list). 其实JDK特意为这种情况准备了一个方法,那就是java.util包中的Array

不断优化，重构我的代码-----拖拽jquery插件

最近学东西学的有点太散了,歇一阵子,重新完善之前的JQ插件,今天先上拖拽吧 // JavaScript Document (function($){ var defaults = { actionElement : "", //获得事件的元素,非必填项 rangeElement : window, //可拖动范围的元素 非必填 direction : "", //默认为空,表示何以任意拖动, 可选值为vertical或者horizontal magnetic : 0

分布式技术追踪 2017年第十七期

分布式系统实践 1. Kudu vs HBase http://dwz.cn/5PF5gq 摘要: Cloudera在2016年发布了新型的分布式存储系统-kudu, kudu从外部形态上来说和HBase非常像, 这篇文章分析了二者的差异和定位. 2. Leaf--美团点评分布式ID生成系统 http://dwz.cn/5PFDFZ 摘要: 又一篇介绍分布式ID生成系统的文章, 思路大体上相同, 一些细节的优化值得学习. 微服务技术 1. 基于Prometheus的数据库监控 http://dw

java重构原则

1.合并条件表达式:一系列表达式得到同一个结果,将这些结果测试合并为一个表达式 * 2.嵌套内的if else和最外层并没有什么关联性,完全可以提取到最顶层,改为平行关系,而非包含.废除 * 临时变量,直接return * 3.减少嵌套和移除临时变量,维持正常流程代码再最外层.将条件反转,使异常状况先退出,让正常流程维持在主干 * 4.箭头型代码,嵌套过深.解决方法是异常条件先退出,保持主干流程是核心流程. * 5.把if-else内的代码都封装成一个公共函数,针对状态处理的代码, * 一种优雅

写if-else不外乎两种场景：异常逻辑处理和不同状态处理。

异常逻辑处理说明只能一个分支是正常流程,而不同状态处理都所有分支都是正常流程. 第一个例子`if (obj != null)`是异常处理,是代码健壮性判断,只有if里面才是正常的处理流程,`else`分支是出错处理流程 //举例一:异常逻辑处理例子 Object obj = getObj(); if (obj != null) { //do something }else{ //do something 第二个例子不管type等于1,2还是其他情况,都属于业务的正常流程.对于这两种情况重构的方法