文档结构  
翻译进度:已翻译     翻译赏金:0 元 (?)    ¥ 我要打赏

这篇文章是一个系列中的一部分,在这个系列里我会调查那些看起来有点可疑的代码(被称为“代码异味”),并且探索一些可行的替代方法。

继续我们的代码异味以及如何处理他们的系列之旅,这篇文章里我来分析一些看起来相当无辜的代码是如何颠覆那些明显的重构。尽管这些例子代码本身会有些琐碎细微,但是它却是我在这个项目里反复都遇到的一个很典型的问题:深度嵌套代码。它可以是循环语句、if语句,甚至是lambda表达式或者内部类,又或者是以上所有的组合体。

第 1 段(可获 1.48 积分)

异味:深层嵌套代码

这段别扭的代码是在一个双重for循环中使用了一个内部的if语句。

public MappedField getMappedField(final String storedName) {
        for (final MappedField mf : persistenceFields) {
            for (final String n : mf.getLoadNames()) {
                if (storedName.equals(n)) {
                    return mf;
                }
            }
        }
        return null;
}

这段代码有什么问题?如果我们可以将就其使用单字符来命名的变量的话,读起来倒也没什么问题,而且我们也已经习惯了使用程序化的方式工作 – 我们遍历所有的字段,然后对于每一个字段查看其名字,如果一个名字跟我们要找的名字匹配上了,那么我们就返回该属性。很简单的逻辑。

第 2 段(可获 1.45 积分)

解决方案1:使用Java 8来帮忙

这里箭头形状部分的代码 看起来有点可疑。我已经在之前的博文谈话中提到过,嵌套的for或者if语句通常都可以用Java的Streams替换掉,而且这样通常能带来更好的可读性。 以前我还一直纳闷,为什么IntelliJ IDEA不建议这种特定的代码可以折叠转换为一个Stream操作,现在看起来可能是因为flatMap/findFirst才是更适合的答案。但是也并没有那么简单。我曾尝试着自己倒腾出一个正确的答案而不借助于IDE的自动帮助,然后就得出了如下的代码:

Optional found = persistenceFields.stream()
                 .flatMap(mappedField -> mappedField.getLoadNames().stream())
                 .filter(storedName::equals)
                 .findFirst();
第 3 段(可获 1.44 积分)

其实这样不好。我想要返回包含该名字的mappedField,而不是一个包含我发现的名字的Optional对象。而且我之后也无法在Stream里面访问到mappedField。 我能做的最好的就是:

persistenceFields.stream()
       .filter(mappedField -> {
            for (String name : mappedField.getLoadNames()) {
               if (storedName.equals(name)) {
                  return true;
               }
            }
            return false;
           })
       .findFirst()
第 4 段(可获 0.84 积分)

呃,有点讨厌。

解决方案2:更好的封装

这里真正的问题是什么? 原先的代码看起来深入到另一个对象的内部,快速浏览,然后只关心顶层对象。移除掉循环语法之后,我们会看到更清晰的内容:

if (getPersistenceFields().get(0).getLoadNames().get(0).equals(storedName))
    return true;

迪米特法则不鼓励我们这样做,因此我们需要考虑一种“命令,不要去询问”的办法。 如果我们问mappedField是否他的其中一个名字与我们想要的名字匹配会不会更好呢?

第 5 段(可获 1.23 积分)
        for (final MappedField mf : persistenceFields) {
            if (mf.hasName(storedName)) {
                return mf;
            }
        }

另外一个遍历名字的for循环被移到了MappedField类里面,这样只有这一个类需要清楚这些名字的存储方式的内部细节。让我们浏览下实现这种重构的一种方法。当然不止这一种方式,这只是一个例子。

首先,抽取这个内部循环成为一个单独的方法。在这个例子中,我们叫它hasName:

Extract Method hasName

抽取方法 hasName

第 6 段(可获 1.09 积分)

抽取出来的方法没法通过编译,不过这没关系,因为我还要修改方法签名的 – 我想让这个方法当这个名字与MappedField的其中一个名字匹配时返回true,否则的话就返回false。

Change method signature to return a boolean

修改方法签名,返回布尔类型值

这样我在原来的方法里可以使用这个返回值

Use the boolean value returned

使用返回的布尔值

代码现在通过编译了,并且和原来的代码行为一致。现在我只需要将hasName方法移到MappedField类中。

Move method into MappedClass

将方法移到MappedClass中

这样MappedField就有了hasName方法:

第 7 段(可获 1.19 积分)
public boolean hasName(String storedName) {
        for (final String name : getLoadNames()) {
            if (storedName.equals(name)) {
                return true;
            }
        }
        return false;
}

…而且这个包含getMappedField方法的类(MappedClass)再也不需要知道MappedField存储其名字的内部细节,或者它甚至可以拥有不止一个名字。

可选的额外步骤: Java 8

如果你愿意的话,这两个方法都可以使用Java 8的语法。对于hasName,这更具有描述性,它标明我们只关心是否有任何名字匹配给定的名字。

第 8 段(可获 1.21 积分)

Java 8 Streams version of hasName

hasName方法可以转换为使用Java 8 Streams。

我们可以这样做:

public boolean hasName(String storedName) {
  return getLoadNames().stream().anyMatch(storedName::equals);
}

同时,getMappedField也可以使用Java 8改造:

Java 8 version of getMappedField

getMappedField方法也可以转换成使用Java 8 Streams

getMappedField代码最终就更像我最初想要的样子了:

public MappedField getMappedField(final String storedName) {
  return persistenceFields.stream()
                          .filter(mf -> mf.hasName(storedName))
                          .findFirst()
                          .orElse(null);
}
第 9 段(可获 1.1 积分)

最后一步(与这个代码异味无关)

当我在寻找适合返回 Optional 返回类型的方法时,我无意中看到了原始代码。在storedName没有匹配任何MappedField时,它返回了null,这看起来像是一个不错的候选方案。如果我们再用上Java 8语法,那么使用Optional就会显得更加清晰易读了

public Optional getMappedField(final String storedName) {
  return persistenceFields
         .stream()
         .filter(mf -> mf.hasName(storedName))
         .findFirst();
}
第 10 段(可获 1.05 积分)

现在我们只需要在调用该方法的地方处理返回的Optional值,不过那就是另外一篇文章所要讲述的主题了...

总结

使用像Java这样的语言时,我们经常会看到多层嵌套的for循环和if语句,特别是在使用Java 8 之前的版本写的代码中。这种代码在处理底层数据结构(比如数组,集合等)时完全是可以接受的,但是如果要是出现在了你的代码中,那就应该被视作代码异味了,特别是当嵌套显得特别深的时候。在我们的例子中,虽然MappedClass类需要了解它的MappedFields,但是它并不需要去遍历MappedField的内部构造来获取数据,它应该让MappedField 让自己处理请求从而得到响应数据。

第 11 段(可获 1.55 积分)

症状:

  • 深层嵌套代码,通常是循环或条件;
  • 全是getter方法的链式方法调用(或者其他非builder、非DSL的方法链)。

潜在的解决方案

  • 考虑告知(tell)而不是询问(ask)。当看到有代码在获取某类的内部数据并对其进行检查时,可以将这些检查移到该类中,并创建一个有意义的名字的方法,可以方便其他对象获取所需数据。
  • 检查你的封装。其数据是否真的需要暴露给其他类吗?数据和其职责是否应该重新分配,这样对于数据的操作可以离数据自身更近一些?
  • 集合的封装。有时这种模式就意味着你正在使用一种数据结构作为一个领域对象。考虑将此数据结构(集合,数组等)封装进一个领域对象内部并添加帮助方法(helper),这样可以让其他对象通过询问得到答案而不是让它们亲自去其内部寻找所需数据。
第 12 段(可获 1.88 积分)

文章评论