消除代码坏味道-阿里云开发者社区

一、背景

开发一款Idea插件，实现对yaml文件的定制化格式检查。

!! 后指定的类路径是否准确
yaml中的key是否equal类中field的name
value是否能够转换成类中field的类型
……

完成代码功能上线后，使用过程发现很多问题。后在主管帮助下，对代码进行了重构。事后对重构前后代码的好坏进行分析总结，文章下面将从结构设计、代码可读性、鲁棒性3个角度对重构前后代码作比较。

二、代码比较

1 结构设计

before：

after：

比较：

after：增加抽象类中的celtVisitMapping层代码，对多个代码检查模块做统一代理。做了错误的捕获，后面也可以做一些其他的统一处理（日志、标识参数等），方便拓展。

2 代码可读性

2.1命名

一个好的命名能输出更多的信息，它会告诉你，它为什么存在，它是做什么事的，应该怎么使用。

2.1.1 类

功能	时间	类名称
检查yaml文件是否可以成功反序列化成项目中的对象。	before	YamlBaseInspection
检查yaml文件是否可以成功反序列化成项目中的对象。	after	CeltClassInspection

比较：

类的命名要做到见名知意，before的命名YamlBaseInspection做不到这一点，通过类名并不能够获取到有用的信息。对于CeltClassInspection的命名格式，在了解插件功能的基础上，可以直接判断出属于yaml类格式检查。

2.1.2 函数

功能	时间	函数名称
比较value是否可以反序列化成PsiClass	before	compareNameAndValue
比较value是否可以反序列化成PsiClass	after	compareKeyAndValue

比较：

before：

1.name是Class中field中的name，通过函数名称并不能够看出，函数名传达信息不准确。

2.Value是yaml中map的概念前后单位不统一。两者放在一起，会使阅读代码者很迷惑。

after：函数名前后单位统一，key和Value是一个yaml中map的两个概念。能从函数名得出函数功能：检验Key和Value的是否准确。

2.1.3 变量

//before
ASTNode node = mapping.getNode().findChildByType(YAMLTokenTypes.TAG);
String className = node.getText().substring(2);
//after
ASTNode node = mapping.getNode().findChildByType(YAMLTokenTypes.TAG);
String tagClassName = node.getText().substring(2);

比较：

String className 来源可以有两个：

1.通过yaml中tag标签在项目中查找得到。

2.PsiClass中的变量类型得出。

after：通过变量名tagClass 可以快速准确的获取变量名属于上述来源中的第一个，能够降低阅读代码的复杂度。 变量名可以传递更多有用的信息。

2.2 注释

2.2.2 注释格式

before 1.无注释 2.有注释不符合规范
after 有注释符合JavaDoc规范

//before
private boolean checkSimpleValue(PsiClass psiClass, PsiElement value)
/**
 * 检查枚举类的value
 * @return
 */
boolean checkEnum(PsiClass psiClass,String text)
//after
/**
 * @param psiClass
 * @param value
 * @return true 正常；false 异常
 */
private boolean checkSimpleValue(PsiClass psiClass, PsiElement value, ProblemsHolder holder)

2.2.3 注释位置

before：

//simple类型，检查keyName 和 value格式
if (PsiClassUtil.isSimpleType(psiClass)) {
//泛型（T）、Object、白名单：不进行检查
} else if (PsiClassUtil.isGenericType(psiClass)) {
//complex类型
} else {
}

after：

// simpleValue 为 null 或者 "null"
if (YamlUtil.isNull(value)) {
}
if (PsiClassUtil.isSimpleType(psiClass)) {
    // simple类型，检查keyName 和 value格式
    checkSimpleValue(psiClass, value, holder);
} else if (PsiClassUtil.isGenericType(psiClass)) {
    //泛型（T）、Object、白名单：不进行检查
} else {
    checkComplexValue(psiClass, value, holder);
}

行内注释应该在解释的代码块内。

2.3 方法抽象

before:

public void compareNameAndValue(PsiClass psiClass, YAMLValue value) {
    //simple类型，检查keyName 和 value格式
    if (PsiClassUtil.isSimpleType(psiClass)) {
    //泛型（T）、Object、白名单：不进行检查
    } else if (PsiClassUtil.isGenericType(psiClass)) {
    //complex类型
    } else {
        Map<String, PsiType> map = new HashMap<>();
        Map<YAMLKeyValue, PsiType> keyValuePsiTypeMap = new HashMap<>();
        //init Map<KeyValue,PsiType>, 注册keyName Error的错误
        PsiField[] allFields = psiClass.getAllFields();
        YAMLMapping mapping = (YAMLMapping) value;
        Collection<YAMLKeyValue> keyValues = mapping.getKeyValues();
        for (PsiField field : allFields) {
            map.put(field.getName(), field.getType());
        }
        for (YAMLKeyValue keyValue : keyValues) {
            if (map.containsKey(keyValue.getName())) {
                keyValuePsiTypeMap.put(keyValue, map.get(keyValue.getName()));
            } else {
                holder.registerProblem(keyValue.getKey(), "找不到这个属性", ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);
            }
        }
        keyValuePsiTypeMap.forEach((yamlKeyValue, psiType) -> {
            //todo:数组类型type 的 check
            if (psiType instanceof PsiArrayType || PsiClassUtil.isCollectionOrMap(PsiTypeUtil.getPsiCLass(psiType, yamlKeyValue))) {
            } else {
                compareNameAndValue(PsiTypeUtil.getPsiCLass(psiType, yamlKeyValue), yamlKeyValue.getValue());
            }
        });
    }
}

after:

public void compareKeyAndValue(PsiClass psiClass, YAMLValue value, ProblemsHolder holder) {
    // simpleValue 为 null 或者 "null"
    if (YamlUtil.isNull(value)) {
        return;
    }
    if (PsiClassUtil.isSimpleType(psiClass)) {
        // simple类型，检查keyName 和 value格式
        checkSimpleValue(psiClass, value, holder);
    } else if (PsiClassUtil.isGenericType(psiClass)) {
        //泛型（T）、Object、白名单：不进行检查
    } else {
        checkComplexValue(psiClass, value, holder);
    }
}
boolean checkComplexValue();

比较：

before: compareNameAndValue方法代码过长，一个屏幕不能浏览整个方法。方法的框架不能够简洁明亮，即要负责判断类型，进行分发处理，还需要负责complex类型的比较，功能耦合。

after：把对complex对象的比较抽离出一个方法，该方法负责进行复杂类型的比较。原方法只负责区分类型，并调用实际的方法比较。能够清晰的看出方法架构，代码后期易维护。

2.4 if复杂判断

before

after

比较：

before：代码中使用复杂的if嵌套，if是造成阅读代码困难的最重要因素之一。if和for循环的嵌套深V嵌套，代码逻辑不清晰，代码维护比较高，拓展复杂。

after：减少了if嵌套，代码理解成本低，代码易维护，易拓展。

3.鲁棒性

3.1 报错信息精准

//before
holder.registerProblem(value, "类型无法转换", ProblemHighlightType.GENERIC_ERROR);
//after
String errorMsg = String.format("cannot find field:%s in class:%s", yamlKeyValue.getName(), psiClass.getQualifiedName());
holder.registerProblem(yamlKeyValue.getKey(), errorMsg, ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);

比较：

before：对于格式检查出的错误提示很随意，只说明了类型无法转换，from是什么？to是什么？都没有说明白，很多有用的信息并没有反馈到用户。用户使用体验会比较差，像是一个完全不成熟的产品。

after：提示无法在class中找到某一个field。并且明确说明了是哪一个field，哪一个class。帮组用户及时准确定位错误并解决。

3.2 代码健壮性（异常处理）

空指针

before：

代码需要考虑异常（空指针、预期之外的场景），下面代码有空指针异常，deleteSqlList可能为null，3行调用会抛出NPE，程序没有捕获处理。

YAMLKeyValue deleteSqlList = mapping.getKeyValueByKey("deleteSQLList");
YAMLSequence sequence = (YAMLSequence) deleteSqlList.getValue();
List<YAMLSequenceItem> items = sequence.getItems();
for (YAMLSequenceItem item : items) {
    if (!DELETE_SQL_PATTERN.matcher(item.getValue().getText()).find()) {
        holder.registerProblem(item.getValue(), "sql error", ProblemHighlightType.GENERIC_ERROR);
    }
}

after：

@Override
public void doVisitMapping(@NotNull YAMLMapping mapping, @NotNull ProblemsHolder holder) {
    ASTNode node = mapping.getNode().findChildByType(YAMLTokenTypes.TAG);
    //取出node
    if (YamlUtil.isNull(node)) {
        return;
    }
    if (node.getText() == null || !node.getText().startsWith("!!")) {
        // throw new RuntimeException("yaml插件监测异常，YAMLQuotedTextImpl text is null或者不是!!开头");
        holder.registerProblem(node.getPsi(), "yaml插件监测异常，YAMLQuotedTextImpl text is null或者不是!!开头", ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);
        return;
    }
    String tagClassName = node.getText().substring(2);
    PsiClass[] psiClasses = ProjectService.findPsiClasses(tagClassName, mapping.getProject());
    if (ArrayUtils.isEmpty(psiClasses)) {
        String errorMsg = String.format("cannot find className = %s", tagClassName);
        holder.registerProblem(node.getPsi(), errorMsg, ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);
        return;
    }
    if (psiClasses.length == 1) {
        compareKeyAndValue(psiClasses[0], mapping, holder);
    }
}

每一步操作都会考虑异常情况，7、11、20行都有对空指针异常的处理。

比较：

after：代码对异常场景考虑更全面，tagString格式非法，空指针，数组越界等等情况。代码更健壮。

switch中的default

before：

switch (className) {
    case "java.lang.Boolean":
        break;
    case "java.lang.Character":
        break;
    case "java.math.BigDecimal":
        break;
    case "java.util.Date":
        break;
    default:
}

after:

switch (className) {
    case "java.lang.Boolean":
        break;
    case "java.lang.Character":
        break;
    case "java.math.BigDecimal":
        break;
    case "java.util.Date":
    case "java.lang.String":
        return true;
    default:
        holder.registerProblem(value, "未识别的className：" +className, ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);
        return false;
}

比较：

before：代码存在隐藏逻辑String类型会走default逻辑不处理，增加代码理解的难度。未对非simple类型的default有异常处理。

after：对String类型写到具体case，暴漏隐藏逻辑。并对default做异常处理，代码更健壮。

消除代码坏味道

一、背景

二、代码比较

1 结构设计

2 代码可读性

2.1命名

2.1.1 类

2.1.2 函数

2.1.3 变量

2.2 注释

2.2.2 注释格式

2.2.3 注释位置

2.3 方法抽象

2.4 if复杂判断

3.鲁棒性

3.1 报错信息精准

3.2 代码健壮性（异常处理）

空指针

switch中的default

热门文章

最新文章

相关电子书

热门

活动广场

任务中心

开发者评测

高校计划

乘风者计划

训练营

阿里云MVP

话题

直播

下载

镜像站

技术资料

插件

消除代码坏味道

一、背景

二、代码比较

1 结构设计

2 代码可读性

2.1命名

2.1.1 类

2.1.2 函数

2.1.3 变量

2.2 注释

2.2.2 注释格式

2.2.3 注释位置

2.3 方法抽象

2.4 if复杂判断

3.鲁棒性

3.1 报错信息精准

3.2 代码健壮性（异常处理）

空指针

switch中的default

热门文章

最新文章

相关电子书