ToB企服应用市场:ToB评测及商务社交产业平台

标题: 案例分析｜如何消除代码坏味道 [打印本页]
作者: 魏晓东    时间: 2023-2-17 13:22
标题: 案例分析｜如何消除代码坏味道

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

完成代码功能上线后，使用过程发现很多问题。后在主管帮助下，对代码进行了重构。事后对重构前后代码的好坏进行分析总结，文章下面将从结构设计、代码可读性、鲁棒性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 变量
  1. //before
  2. ASTNode node = mapping.getNode().findChildByType(YAMLTokenTypes.TAG);
  3. String className = node.getText().substring(2);
  5. //after
  6. ASTNode node = mapping.getNode().findChildByType(YAMLTokenTypes.TAG);
  7. String tagClassName = node.getText().substring(2);
复制代码
比较：
String className 来源可以有两个：
1.通过yaml中tag标签在项目中查找得到。
2.PsiClass中的变量类型得出。after：通过变量名 tagClass 可以快速准确的获取变量名属于上述来源中的第一个，能够降低阅读代码的复杂度。变量名可以传递更多有用的信息。
2.2 注释

2.2.1 注释格式

  1. //before
  3. private boolean checkSimpleValue(PsiClass psiClass, PsiElement value)
  5. /**
  6. * 检查枚举类的value
  7. * @return
  8. */
  9. boolean checkEnum(PsiClass psiClass,String text)
  11. //after
  12. /**
  13. * @param psiClass
  14. * @param value
  15. * @return true 正常；false 异常
  16. */
  17. private boolean checkSimpleValue(PsiClass psiClass, PsiElement value, ProblemsHolder holder)
复制代码
2.2.2 注释位置

before：
  1. //simple类型，检查keyName 和 value格式
  2. if (PsiClassUtil.isSimpleType(psiClass)) {
  4. //泛型（T）、Object、白名单：不进行检查
  5. } else if (PsiClassUtil.isGenericType(psiClass)) {
  7. //complex类型
  8. } else {
  10. }
复制代码
 
after：
  1. // simpleValue 为 null 或者 "null"
  2. if (YamlUtil.isNull(value)) {
  4. }
  5. if (PsiClassUtil.isSimpleType(psiClass)) {
  6.     // simple类型，检查keyName 和 value格式
  7.     checkSimpleValue(psiClass, value, holder);
  8. } else if (PsiClassUtil.isGenericType(psiClass)) {
  9.     //泛型（T）、Object、白名单：不进行检查
  10. } else {
  11.     checkComplexValue(psiClass, value, holder);
  12. }
复制代码
  1.  
复制代码
行内注释应该在解释的代码块内。
2.3 方法抽象

before:
  1. public void compareNameAndValue(PsiClass psiClass, YAMLValue value) {
  2.     //simple类型，检查keyName 和 value格式
  3.     if (PsiClassUtil.isSimpleType(psiClass)) {
  4.     //泛型（T）、Object、白名单：不进行检查
  5.     } else if (PsiClassUtil.isGenericType(psiClass)) {
  7.     //complex类型
  8.     } else {
  9.         Map<String, PsiType> map = new HashMap<>();
  10.         Map<YAMLKeyValue, PsiType> keyValuePsiTypeMap = new HashMap<>();
  11.         //init Map<KeyValue,PsiType>, 注册keyName Error的错误
  12.         PsiField[] allFields = psiClass.getAllFields();
  13.         YAMLMapping mapping = (YAMLMapping) value;
  14.         Collection<YAMLKeyValue> keyValues = mapping.getKeyValues();
  15.         for (PsiField field : allFields) {
  16.             map.put(field.getName(), field.getType());
  17.         }
  18.         for (YAMLKeyValue keyValue : keyValues) {
  19.             if (map.containsKey(keyValue.getName())) {
  20.                 keyValuePsiTypeMap.put(keyValue, map.get(keyValue.getName()));
  21.             } else {
  22.                 holder.registerProblem(keyValue.getKey(), "找不到这个属性", ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);
  23.             }
  24.         }
  25.         keyValuePsiTypeMap.forEach((yamlKeyValue, psiType) -> {
  26.             //todo:数组类型type 的 check
  27.             if (psiType instanceof PsiArrayType || PsiClassUtil.isCollectionOrMap(PsiTypeUtil.getPsiCLass(psiType, yamlKeyValue))) {
  28.          
  29.             } else {
  30.                 compareNameAndValue(PsiTypeUtil.getPsiCLass(psiType, yamlKeyValue), yamlKeyValue.getValue());
  31.             }
  32.         });
  33.     }
  34. }
复制代码
  1.  
复制代码
after:
  1. public void compareKeyAndValue(PsiClass psiClass, YAMLValue value, ProblemsHolder holder) {
  2.     // simpleValue 为 null 或者 "null"
  3.     if (YamlUtil.isNull(value)) {
  4.         return;
  5.     }
  6.   
  7.     if (PsiClassUtil.isSimpleType(psiClass)) {
  8.       
  9.         // simple类型，检查keyName 和 value格式
  10.         checkSimpleValue(psiClass, value, holder);
  11.   
  12.     } else if (PsiClassUtil.isGenericType(psiClass)) {
  13.       
  14.         //泛型（T）、Object、白名单：不进行检查
  15.     } else {
  16.         checkComplexValue(psiClass, value, holder);
  17.     }
  18. }
  19. boolean checkComplexValue();
复制代码
  1.  
复制代码
比较：
before: compareNameAndValue方法代码过长，一个屏幕不能浏览整个方法。方法的框架不能够简洁明亮，即要负责判断类型，进行分发处理，还需要负责complex类型的比较，功能耦合。
after：把对complex对象的比较抽离出一个方法，该方法负责进行复杂类型的比较。原方法只负责区分类型，并调用实际的方法比较。能够清晰的看出方法架构，代码后期易维护。
2.4 if复杂判断

before
after

比较：before：代码中使用复杂的if嵌套，if是造成阅读代码困难的最重要因素之一。if和for循环的嵌套深V嵌套，代码逻辑不清晰，代码维护比较高，拓展复杂。
after：减少了if嵌套，代码理解成本低，代码易维护，易拓展。
3.鲁棒性
3.1 报错信息精准
  1. //before
  2. holder.registerProblem(value, "类型无法转换", ProblemHighlightType.GENERIC_ERROR);
  5. //after
  6. String errorMsg = String.format("cannot find field:%s in class:%s", yamlKeyValue.getName(), psiClass.getQualifiedName());
  8. holder.registerProblem(yamlKeyValue.getKey(), errorMsg, ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);
复制代码
  1.  
复制代码
比较：before：对于格式检查出的错误提示很随意，只说明了类型无法转换，from是什么？to是什么？都没有说明白，很多有用的信息并没有反馈到用户。用户使用体验会比较差，像是一个完全不成熟的产品。
after：提示无法在class中找到某一个field。并且明确说明了是哪一个field，哪一个class。帮组用户及时准确定位错误并解决。
3.2 代码健壮性（异常处理）

空指针

before：
代码需要考虑异常（空指针、预期之外的场景），下面代码有空指针异常，deleteSqlList可能为null，3行调用会抛出NPE，程序没有捕获处理。
 
  1. YAMLKeyValue deleteSqlList = mapping.getKeyValueByKey("deleteSQLList");
  2. YAMLSequence sequence = (YAMLSequence) deleteSqlList.getValue();
  3. List<YAMLSequenceItem> items = sequence.getItems();
  4. for (YAMLSequenceItem item : items) {
  5.     if (!DELETE_SQL_PATTERN.matcher(item.getValue().getText()).find()) {
  6.         holder.registerProblem(item.getValue(), "sql error", ProblemHighlightType.GENERIC_ERROR);
  7.     }
  8. }
复制代码
  1.  
复制代码
after：
  1. @Override
  2. public void doVisitMapping(@NotNull YAMLMapping mapping, @NotNull ProblemsHolder holder) {
  3.    
  4.     ASTNode node = mapping.getNode().findChildByType(YAMLTokenTypes.TAG);
  5.    
  6.     //取出node
  7.     if (YamlUtil.isNull(node)) {
  8.         return;
  9.     }
  10.   
  11.     if (node.getText() == null || !node.getText().startsWith("!!")) {
  12.         // throw new RuntimeException("yaml插件监测异常，YAMLQuotedTextImpl text is null或者不是!!开头");
  13.         holder.registerProblem(node.getPsi(), "yaml插件监测异常，YAMLQuotedTextImpl text is null或者不是!!开头", ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);
  14.         return;
  15.     }
  16.     String tagClassName = node.getText().substring(2);
  17.     PsiClass[] psiClasses = ProjectService.findPsiClasses(tagClassName, mapping.getProject());
  18.     if (ArrayUtils.isEmpty(psiClasses)) {
  19.         String errorMsg = String.format("cannot find className = %s", tagClassName);
  20.         holder.registerProblem(node.getPsi(), errorMsg, ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);
  21.         return;
  22.     }
  23.    
  24.     if (psiClasses.length == 1) {
  25.         compareKeyAndValue(psiClasses[0], mapping, holder);
  26.     }
  27. }
复制代码
每一步操作都会考虑异常情况，7、11、20行都有对空指针异常的处理。比较：
after：代码对异常场景考虑更全面，tagString格式非法，空指针，数组越界等等情况。代码更健壮。
switch中的default

before：
  1. switch (className) {
  2.     case "java.lang.Boolean":
  3.         break;
  4.     case "java.lang.Character":
  5.         break;
  6.     case "java.math.BigDecimal":
  7.         break;
  8.     case "java.util.Date":
  9.             break;
  10.     default:
  11. }
复制代码
after:
  1. switch (className) {
  2.     case "java.lang.Boolean":
  3.         break;
  4.     case "java.lang.Character":
  5.         break;
  6.     case "java.math.BigDecimal":
  7.         break;
  8.             case "java.util.Date":
  9.     case "java.lang.String":
  10.         return true;
  11.     default:
  12.         holder.registerProblem(value, "未识别的className：" +className, ProblemHighlightType.LIKE_UNKNOWN_SYMBOL);
  13.         return false;
  14. }
复制代码
比较：
before：代码存在隐藏逻辑String类型会走default逻辑不处理，增加代码理解的难度。未对非simple类型的default有异常处理。
after：对String类型写到具体case，暴漏隐藏逻辑。并对default做异常处理，代码更健壮。
 
作者|王耀兴(承録)

免责声明：如果侵犯了您的权益，请联系站长，我们会及时删除侵权内容，谢谢合作！



欢迎光临 ToB企服应用市场:ToB评测及商务社交产业平台 (https://dis.qidao123.com/) Powered by Discuz! X3.4