代码审查|这段代码,为什么复制文件夹总是“成功”?

简介: 最近开始一个人负责整个项目的全栈开发和维护,工作中没了和同事交叉 code review 的环节,所以就打算,如果工作中遇到一些比较典型的代码,包括好味道和坏味道,就拿出来分析下,与大家一起交流,作为另一种形式的「交叉 review」。

最近开始一个人负责整个项目的全栈开发和维护,工作中没了和同事交叉 code review 的环节,所以就打算,如果工作中遇到一些比较典型的代码,包括好味道和坏味道,就拿出来分析下,与大家一起交流,作为另一种形式的「交叉 review」。

这天遇到这样一个问题:在 Android 手机上复制 assets 里的文件夹到手机里,实际并没有拷贝完成,但代码总是显示成功,看了下代码,使用的是阿里云播放器 Android SDK 的 Demo 里的一个工具类。

工具类里的相关代码经过简化后示意如下:

public class Commen {
    private static Commen instance;
    private volatile boolean isSuccess;

    public static Commen getInstance(Context context) {
        // some code here,单例控制,返回 instance
        // ...
    }

    public void copyAssetsToDst(Context context, String srcPath, String dstPath) {
        try {
            String[] fileNames = context.getAssets().list(srcPath);
            if (fileNames.length > 0) {
                for (String fileName : fileNames) {
                    // 文件夹,递归调用
                    copyAssetsToDst(context, srcPath + File.separator + fileName,
                            dstPath + File.separator + fileName);
                }
            } else {
                // some code here,单个文件拷贝
                // ...
            }
            isSuccess = true;
        } catch (Exception e) {
            isSuccess = false;
        }
    }
}

这段代码使用起来若不谨慎,至少存在以下问题:

  1. 线程安全问题:该类是一个单例类,代码中的 isSuccess 相当于是一个全局变量,如果多个线程同时调用 copyAssetsToDst 方法,会出现线程安全问题,导致 isSuccess 的值被交叉覆盖,不可预期;

  2. 结果正确性:因为 Exception 全都被 catch 住了,这样如果 srcPath 是一个文件夹,递归调用方法自身后,最外层总是会将 isSuccess 设置为 true,导致最终结果总是显示成功,而实际结果未知。

如果由我来写这段代码,我会做这样的修改:

  • 将类改为工具类,公开的方法都是静态方法,不需要单例控制;

  • 方法执行是否成功,由返回值、是否抛出异常来表示,不使用成员变量记录;

  • 拷贝过程中,记录拷贝成功的文件列表,如果最终失败,将中间生成的文件做清理。


如果读完文章有收获,可以关注我的微信公众号「闷骚的程序员」并🌟设为星标🌟,随时阅读更多内容。

目录
相关文章
|
机器学习/深度学习 人工智能 自然语言处理
从此告别PPT制作的烦恼:ChatGPT和MindShow帮你快速完成
从此告别PPT制作的烦恼:ChatGPT和MindShow帮你快速完成
|
12月前
|
人工智能 自然语言处理 自动驾驶
阿里云入选Gartner® AI代码助手魔力象限挑战者象限
Gartner发布业界首个AI代码助手魔力象限,全球共12家企业入围,阿里云,成为唯一进入挑战者象限的中国科技公司。对阿里云而言,此次入选代表了其通义灵码在产品功能和市场应用等方面的优秀表现。
|
存储 监控 关系型数据库
InfluxDB入门:基础概念解析
【4月更文挑战第30天】InfluxDB是开源时序数据库,擅长处理实时数据,常用于监控和分析。本文介绍了其基础概念:数据库(数据容器)、测量值(类似表)、字段(数据值)、标签(元数据)、时间戳和数据点。InfluxDB特性包括高性能写入、灵活查询(InfluxQL和Flux)、可扩展性及活跃社区支持。了解这些概念有助于更好地使用InfluxDB处理时间序列数据。
|
12月前
|
存储 JavaScript 前端开发
如何使用Vue.js实现一个简单的待办事项应用
【10月更文挑战第1天】如何使用Vue.js实现一个简单的待办事项应用
284 5
|
12月前
|
存储 设计模式 缓存
从一个 NullPointerException 探究 Java 的自动装箱拆箱机制
这行代码一个对象方法都没有调用,怎么会抛出 NullPointerException 呢?
115 9
|
12月前
|
Java 程序员 编译器
Java|如何正确地在遍历 List 时删除元素
从源码分析如何正确地在遍历 List 时删除元素。为什么有的写法会导致异常,而另一些不会。
279 3
|
12月前
|
程序员
后端|一个分布式锁「失效」的案例分析
小猿最近很苦恼:明明加了分布式锁,为什么并发还是会出问题呢?
120 2
|
Cloud Native Java 数据库
深入理解Micronaut依赖注入:提高应用灵活性的最佳实践
【9月更文挑战第5天】Micronaut是一个轻量级全栈业务框架,支持Java与Groovy语言,以其优秀的性能和对云原生特性的深度集成而备受关注。本文探讨Micronaut中的依赖注入机制,通过示例展示如何利用构造函数注入、字段注入及方法注入等方式提高应用灵活性。通过合理的依赖注入策略,如使用`@Qualifier`注解选择具体实现或条件化注册Bean,可构建更易扩展和维护的应用。Micronaut简化了这一过程,使开发者能专注于业务逻辑。
175 2
|
12月前
|
存储 安全 程序员
读书|通过免费云盘传书到 Kindle
通过免费云盘便捷地管理 Kindle 上的书籍。
249 0
|
数据可视化 IDE Java
PlanUML和Mermaid哪个好?
PlanUML和Mermaid哪个好?
2485 0