0%

问题概述

当我们发现一个过长的方法时,通常会想要将之拆分为几个小方法.在某些情况下这非常容易,例如:一个很长的方法有很多参数,这些参数可以明显分为两组,一组只在方法的前一半使用,另一组在后一半使用.可以很明确地知道在什么位置将长方法拆成两个小方法.

有类似这种表现的方法通常都具有多重职责,逻辑十分复杂.在上面的举例中,虽然可以很明确知道在什么位置将代码拆开,但是可能前半部分会有多个返回值,需要传递给后半部分.

复杂的方法不仅逻辑很难理解,也很难重构,几乎不可能重用.后续的维护多半只能是重写.

症状

  • 方法参数可以分组,分别在方法代码中不重合的段落中使用.例如三个用在前半部分,两个用在后半部分.
  • Boolean类型的参数用来控制方法中某些代码块的开关.
  • 方法中的一部分只是用来计算一个值供另外一部分使用.

可能的解决方案

  • 将明显的代码块拆分为单独的方法(例如一个检查Boolean类型参数的if语句块)
  • 如果两个构造函数没有重合的参数,且非常复杂.那么可以将这个类拆分为两个单独的类.

示例代码

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
static ValidatedField validateQuery(Class clazz, Mapper mapper, String propertyName, FilterOperator op,
                                    Object val, boolean validateNames, boolean validateTypes) {
    final ValidatedField validatedField = new ValidatedField(propertyName);
    if (!propertyName.startsWith("$")) {
        final String[] pathElements = propertyName.split("\\.");
        final List<String> databasePathElements = new ArrayList<>(asList(pathElements));
        if (clazz == null) {
            return validatedField;
        }

        MappedClass mc = mapper.getMappedClass(clazz);
        for (int i = 0; ; ) {
            final String fieldName = pathElements[i];
            boolean fieldIsArrayOperator = fieldName.equals("$");
           final Optional<MappedField> mf = getMappedField(fieldName, mc, databasePathElements,
                                                            i, propertyName, validateNames,
                                                            fieldIsArrayOperator);
            validatedField.mappedField = mf;

            i++;
            if (mf.isPresent() && mf.get().isMap()) {
                //skip the map key validation, and move to the next fieldName
                i++;
            }
            if (i >= pathElements.length) {
                break;
            }
           if (!fieldIsArrayOperator) {
                //catch people trying to search/update into @Reference/@Serialized fields
                if (validateNames && !canQueryPast(mf.get())) {
                    throw new ValidationException(
format("Cannot use dot-notation past '%s' in '%s'; found while validating - %s",
fieldName, mc.getClazz().getName(), propertyName));
                }
                if (!mf.isPresent() && mc.isInterface()) {
                    break;
                } else if (!mf.isPresent()) {
                    throw new ValidationException(
format("The field '%s' could not be found in '%s'", propertyName,
mc.getClazz().getName()));
                }
                //get the next MappedClass for the next field validation
                MappedField mappedField = mf.get();
                mc = mapper.getMappedClass((mappedField.isSingleValue())
? mappedField.getType()
: mappedField.getSubClass());
            }
        }
        //record new property string
        validatedField.databasePath = databasePathElements.stream().collect(joining("."));

        if (validateTypes && validatedField.mappedField.isPresent()) {
            MappedField mappedField = validatedField.mappedField.get();
            List<ValidationFailure> typeValidationFailures = new ArrayList<>();
            boolean compatibleForType = isCompatibleForOperator(
mc, mappedField, mappedField.getType(), op, val, typeValidationFailures);
            List<ValidationFailure> subclassValidationFailures = new ArrayList<>();
            boolean compatibleForSubclass = isCompatibleForOperator(
mc, mappedField, mappedField.getSubClass(), op, val, subclassValidationFailures);

            if ((mappedField.isSingleValue() && !compatibleForType)
                 || mappedField.isMultipleValues() && !(compatibleForSubclass || compatibleForType)) {
                if (LOG.isWarningEnabled()) {
                    LOG.warning(
format("The type(s) for the query/update may be inconsistent; "
+"using an instance of type '%s' for the field '%s.%s'"
+" which is declared as '%s'",
val.getClass().getName(), mappedField.getDeclaringClass().getName(),
mappedField.getJavaFieldName(), mappedField.getType().getName()));
                    typeValidationFailures.addAll(subclassValidationFailures);
                    LOG.warning("Validation warnings: \n" + typeValidationFailures);
                }
            }
        }
    }
    return validatedField;
}

第一步,观察11-48行,这段代码计算了MapperClass mc的值供后面的代码使用,可以拆解出来.

问题概述

在Java中,默认情况下很多东西是可变的.

  • 类的字段导致对象状态是可变的
  • 方法的参数/局部变量可以重复赋值
  • 集合容器的内容可变

这些可变性,导致方法的逻辑复杂性很难理解并限制了并发性.

症状

  • 对变量不只一次赋值
  • 修改方法参数的状态(重新赋值,或修改对象内部状态)
  • 为改变参数内容而传递包装类型(例如:StringBuilder,可变的集合等等)
  • 由于代码中有多个原始类型变量被更新,导致很难做方法提取重构
  • 使用计数器或布尔类型变量跟踪分支代码的状态变更

可能的解决方案

  • 如果代码在从一个集合或数组中读取内容的同时修改它,那最好使用另外一个数组或集合来跟踪变更的内容,保持原始的数组或集合内容不变.同时读写一个集合会导致意外的结果,并带来并发问题.
  • 如果很难解释一个变量的值是在哪个分支中被赋予的,那么最好用多个具有良好命名的变量,而不是重用同一个.又或者将变更变量的代码拆分为多个小方法,从而将变更限制在小段代码中.
  • 方法最好使用返回值来向调用者传递消息,而不是通过修改输入参数对象告知调用者自己的结果.如果需要传递的信息比较复杂,可以声明一个包装类来维护所有需要返回的信息.
  • 如果方法中使用了计数器或布尔类型变量来跟踪状态,那么最好将这类变量移动到合适的业务领域对象中,以便于方法可以进行简单的重构.

Null表示什么?

我们的代码用Null表示了太多的事情:

  • 有意或无意导致的从未初始化的值
  • 无效值/不需要的值
  • 值不存在(比如从数据库中查找不到)
  • 可能出现了严重的错误导致本该存在的值没有了
  • 其他各种情况

由于上述种种,NPE总是时不时出现在我们的程序中.NPE导致的BUG也数不胜数,没见过NPE的程序员估计还没出生.

Null是个尤其难解决的问题,主要原因是开发人员很难明确的知道Null表达的实际意义.从字面上看Null表示没有任何含义的值.但用它来表达上面的任何情况都不足够清楚.

症状

当Null在你的程序里面成为顽疾,往往会有如下症状.

  • 随处可见的Null检查if(xx != null).

    这种情况说明开发人员不清楚传递的变量为null的意义,只能无奈的做防御式Null检查.

    开发人员永远不知道返回值是否可能为null,也永远无法保证在所有地方都使用的Null检查.

  • 方法直接使用return null

可能的解决方案

  • Java8+或guava的Optional,它虽然不是处理一切Null问题的银弹,但是当一个public方法通过明确返回null来表示效果不存在时,使用Optional是更好的方案.
  • @NotNull/@Nullable在Intellij,guava,lombok,jsr305中都提供了这样的注解,Intellij IDEA对这些注解的提供了编码时检查.在IDE里面查看一个方法,将@NotNull临时添加在参数前,IDE会告知你该参数是否有被传递null值,如果没有,那么你可以去掉该注解,然后在Javadoc中加入参数不能为null的说明.当然如果你愿意保留@NotNull注解,可以更显著地提醒代码的维护人员,但是需要引入一个注解依赖.
  • 在异常处理中返回null是非常不友好的,调用者往往不希望收到null值,进而导致NPE.整个过程缺少对错误信息的描述.应当改为抛出一个具有描述信息的异常,这对发现错误和分析问题更有价值.

总结

如果方法可能返回null,那么考虑改为返回Optional.

方法的参数通过@NotNull/@Nullable来告知调用者是否允许传递null值,同时为IDE的静态代码分析提供帮助.

异常处理中永远不要返回null值,改为抛出具有描述信息的异常.

概述

代码的异味有很多种,而且通常一份代码中会同时出现多种异味,要重构这样的代码是很难的.

对这样的代码进行重构,通常可以通过以下三个步骤来

  1. 将方法拆分多个相对更短的代码段
  2. 针对每个方法处理异味,但一次仅针对一种异味
  3. 重复前两步,如果前两部难以实施,考虑对你的代码要解决的问题进行重新建模

症状

  • 非常长或复杂的方法或类.
  • 当你大概流量代码时能够发现多种异味.
  • 凭直觉就感觉到代码丑陋.(充满异味的代码通常给令读者心情糟糕)

可能的解决方案

  • 将代码拆分为多个段落
  • 一次仅尝试修复一种异味
  • 引入新的业务领域对象来对代码段解决的问题进行建模
  • 提取出非常小的方法,并进行良好的命名.通过方法名来对代码进行文档化,如果方法名无法描述做了什么事儿,那么为方法补充一段注释是更好的.提取出方法+注释比直接在代码原地注释要更好,因为代码原地注释会拉长代码的篇幅,影响阅读体验.而IDE可以在任何引用该方法的位置通过快捷键快速读取良好格式化后的方法注释.