草庐IT

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

轻风博客 2023-04-16 原文

 

一、背景

开发一款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做异常处理，代码更健壮。

 

作者|王耀兴(承録)

案例分析案例sectiondatadata-cangjie-keyJava

有关案例分析｜如何消除代码坏味道的更多相关文章

  1. ruby - 如何使用 Nokogiri 的 xpath 和 at_xpath 方法 - 2

    我正在学习如何使用Nokogiri，根据这段代码我遇到了一些问题:require'rubygems'require'mechanize'post_agent=WWW::Mechanize.newpost_page=post_agent.get('http://www.vbulletin.org/forum/showthread.php?t=230708')puts"\nabsolutepathwithtbodygivesnil"putspost_page.parser.xpath('/html/body/div/div/div/div/div/table/tbody/tr/td/div

  2. ruby - 如何从 ruby​​ 中的字符串运行任意对象方法？ - 2

    总的来说，我对ruby​​还比较陌生，我正在为我正在创建的对象编写一些rspec测试用例。许多测试用例都非常基础，我只是想确保正确填充和返回值。我想知道是否有办法使用循环结构来执行此操作。不必为我要测试的每个方法都设置一个assertEquals。例如:describeitem,"TestingtheItem"doit"willhaveanullvaluetostart"doitem=Item.new#HereIcoulddotheitem.name.shouldbe_nil#thenIcoulddoitem.category.shouldbe_nilendend但我想要一些方法来使用

  3. python - 如何使用 Ruby 或 Python 创建一系列高音调和低音调的蜂鸣声？ - 2

    关闭。这个问题是opinion-based.它目前不接受答案。想要改进这个问题？更新问题，以便editingthispost可以用事实和引用来回答它.关闭4年前。Improvethisquestion我想在固定时间创建一系列低音和高音调的哔哔声。例如:在150毫秒时发出高音调的蜂鸣声在151毫秒时发出低音调的蜂鸣声200毫秒时发出低音调的蜂鸣声250毫秒的高音调蜂鸣声有没有办法在Ruby或Python中做到这一点？我真的不在乎输出编码是什么(.wav、.mp3、.ogg等等)，但我确实想创建一个输出文件。

  4. ruby-on-rails - 如何验证 update_all 是否实际在 Rails 中更新 - 2

    给定这段代码defcreate@upgrades=User.update_all(["role=?","upgraded"],:id=>params[:upgrade])redirect_toadmin_upgrades_path,:notice=>"Successfullyupgradeduser."end我如何在该操作中实际验证它们是否已保存或未重定向到适当的页面和消息？ 最佳答案 在Rails3中，update_all不返回任何有意义的信息，除了已更新的记录数(这可能取决于您的DBMS是否返回该信息)。http://ar.ru

  5. ruby-on-rails - 'compass watch' 是如何工作的/它是如何与 rails 一起使用的 - 2

    我在我的项目目录中完成了compasscreate.和compassinitrails。几个问题:我已将我的.sass文件放在public/stylesheets中。这是放置它们的正确位置吗？当我运行compasswatch时，它不会自动编译这些.sass文件。我必须手动指定文件:compasswatchpublic/stylesheets/myfile.sass等。如何让它自动运行？文件ie.css、print.css和screen.css已放在stylesheets/compiled。如何在编译后不让它们重新出现的情况下删除它们？我自己编译的.sass文件编译成compiled/t

  6. ruby - 如何将脚本文件的末尾读取为数据文件(Perl 或任何其他语言) - 2

    我正在寻找执行以下操作的正确语法(在Perl、Shell或Ruby中):#variabletoaccessthedatalinesappendedasafileEND_OF_SCRIPT_MARKERrawdatastartshereanditcontinues. 最佳答案 Perl用__DATA__做这个:#!/usr/bin/perlusestrict;usewarnings;while(){print;}__DATA__Texttoprintgoeshere 关于ruby-如何将脚

  7. ruby - 如何在 buildr 项目中使用 Ruby 代码？ - 2

    如何在buildr项目中使用Ruby？我在很多不同的项目中使用过Ruby、JRuby、Java和Clojure。我目前正在使用我的标准Ruby开发一个模拟应用程序，我想尝试使用Clojure后端(我确实喜欢功能代码)以及JRubygui和测试套件。我还可以看到在未来的不同项目中使用Scala作为后端。我想我要为我的项目尝试一下buildr(http://buildr.apache.org/)，但我注意到buildr似乎没有设置为在项目中使用JRuby代码本身!这看起来有点傻，因为该工具旨在统一通用的JVM语言并且是在ruby中构建的。除了将输出的jar包含在一个独特的、仅限ruby​​

  8. ruby - 如何指定 Rack 处理程序 - 2

    Rackup通过Rack的默认处理程序成功运行任何Rack应用程序。例如:classRackAppdefcall(environment)['200',{'Content-Type'=>'text/html'},["Helloworld"]]endendrunRackApp.new但是当最后一行更改为使用Rack的内置CGI处理程序时，rackup给出“NoMethodErrorat/undefinedmethod`call'fornil:NilClass”:Rack::Handler::CGI.runRackApp.newRack的其他内置处理程序也提出了同样的反对意见。例如Rack

  9. ruby-on-rails - Rails 源代码 : initialize hash in a weird way? - 2

    在rails源中:https://github.com/rails/rails/blob/master/activesupport/lib/active_support/lazy_load_hooks.rb可以看到以下内容@load_hooks=Hash.new{|h,k|h[k]=[]}在IRB中，它只是初始化一个空哈希。和做有什么区别@load_hooks=Hash.new 最佳答案 查看rubydocumentationforHashnew→new_hashclicktotogglesourcenew(obj)→new_has

  10. ruby - 如何每月在 Heroku 运行一次 Scheduler 插件？ - 2

    在选择我想要运行操作的频率时，唯一的选项是“每天”、“每小时”和“每10分钟”。谢谢!我想为我的Rails3.1应用程序运行调度程序。 最佳答案 这不是一个优雅的解决方案，但您可以安排它每天运行，并在实际开始工作之前检查日期是否为当月的第一天。 关于ruby-如何每月在Heroku运行一次Scheduler插件？，我们在StackOverflow上找到一个类似的问题： https://stackoverflow.com/questions/8692687/

随机推荐