魏晓东 发表于 2023-2-17 13:22:21

案例分析|如何消除代码坏味道

https://img2023.cnblogs.com/blog/27422/202302/27422-20230209204103592-441833058.png
 
一、背景
开发一款Idea插件,实现对yaml文件的定制化格式检查。

[*]!! 后指定的类路径是否准确
[*]yaml中的key是否equal类中field的name
[*]value是否能够转换成类中field的类型
[*]……
https://mmbiz.qpic.cn/mmbiz_png/Z6bicxIx5naIoUZKb96lIiaZSLdfdn6yrWPqxUXjKRCpRBfNlMAQEKB8XLHQ8YjiaZsKZIyzlsm7YACcQ7VGSlMIA/640?wx_fmt=png
完成代码功能上线后,使用过程发现很多问题。后在主管帮助下,对代码进行了重构。事后对重构前后代码的好坏进行分析总结,文章下面将从结构设计、代码可读性、鲁棒性3个角度对重构前后代码作比较。二、代码比较
1 结构设计
before:
https://mmbiz.qpic.cn/mmbiz_png/Z6bicxIx5naIoUZKb96lIiaZSLdfdn6yrWZiaIAmibfZwK3pxiaGAbWRUmrgtUrdQTpmOLqAm4Ty1iaPcE2zUBYaD05A/640?wx_fmt=png
after:
https://mmbiz.qpic.cn/mmbiz_png/Z6bicxIx5naIoUZKb96lIiaZSLdfdn6yrWicNLNQQGhtApsfJAGtYlJjBWHdlF3dPKoYsmvmoTWQdZR1wXgLYHgLA/640?wx_fmt=png
比较: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复杂判断

beforehttps://img2023.cnblogs.com/blog/27422/202302/27422-20230209204808443-1736987777.png
after
https://img2023.cnblogs.com/blog/27422/202302/27422-20230209204817440-138919006.png
比较: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, 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]
查看完整版本: 案例分析|如何消除代码坏味道