Code Smell: Multi-Responsibility Methods : 多职责方法

Posted on Edited on

问题概述

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

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

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

症状

  • 方法参数可以分组,分别在方法代码中不重合的段落中使用.例如三个用在前半部分,两个用在后半部分.
  • 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的值供后面的代码使用,可以拆解出来.