代码出问题了怎么办（案例分析｜如何消除代码坏味道）

一              、背景

开发一款Idea插件 
  
  
  
  
  
  
  ，实现对yaml文件的定制化格式检查              。 !! 后指定的类路径是否准确 yaml中的key是否equal类中field的name value是否能够转换成类中field的类型 …… 完成代码功能上线后
  
  
  
  
  
  
  
，使用过程发现很多问题 
  
 
  
 
  
 
  
 
  
 
  
 
  
 。后在主管帮助下
 


 


 


 


 


 


 

，对代码进行了重构














。事后对重构前后代码的好坏进行分析总结 
 
 
 
 
 
 
 ，文章下面将从结构设计 
  
 
  
 
  
 
  
 
  
 
  
 
  
 、代码可读性














、鲁棒性3个角度对重构前后代码作比较 
  
  
  
  
  
  
 。

二 
  
  
  
  
  
  
 、代码比较

1结构设计

before：

after：

比较：

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

 

 

 

 

 

 

 ，方便拓展

 


 


 


 


 


 


 

。

2 代码可读性

2.1命名

一个好的命名能输出更多的信息              ，它会告诉你

 
 

 
 

 
 

 
 

 
 

 
 

 
 
，它为什么存在













，它是做什么事的                      ，应该怎么使用 
 
 
 
 
 
 
。

2.1.1 类

功能

时间

类名称

检查yaml文件是否可以成功反序列化成项目中的对象 
   
   
   
   
   
   
   。

before

YamlBaseInspection

after

CeltClassInspection

比较：

类的命名要做到见名知意
 
 
 
 
 
 
 
 
 
 
 
 
 
 
，before的命名YamlBaseInspection做不到这一点




















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

 

 

 

 

 

 

。

2.1.2 函数

功能

时间

函数名称

比较value是否可以反序列化成PsiClass

before

compareNameAndValue

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.1 注释格式 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.2 注释位置

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做异常处理

 
 

 
 

 
 

 
 

 
 

 
 

 
 
，代码更健壮
 

 

 

 

 

 

 
。

作者|王耀兴(承録)