总结开源项目中的常见坏实践（Bad Practice）

[ngduncent]

一些开源项目包含了各种编程的最佳实践供人参考学习和借鉴。但是也有一些开源项目虽然初衷是好的。但是包含了一些代码的坏实践。特别是对于一部分刚入行的大学生来说，可能会给到一些错误的示范。于是在此列举一些项目中的坏实践。

1.方法的用意判断是与否却返回字符串的“0”或者“1”

如果一个方法明确返回是与否这两种情况，那么没有必要返回字符串的 0 或者 1 。这样会造成很多地方需要使用 字符串的形式来匹配结果判断是与否。例如以下这种形式。 方法应该直接返回 true 或者 false 。 代码会简洁明了很多。

2.滥用三元运算符

3.滥用机翻英语

Poor 有不好，差的意思。 例如 My english is poor. 这里的 Poor 是差的意思。 但是下图这个方法 getDatePoor 。也用 poor 来表达获取时间“差”的含义。 英语还是程序员应该要掌握学习的。不能光高靠机翻英语，不然容易闹笑话。

4.造不必要的大量轮子

很多方法或者功能我们应该尽可能的搜索是否已经有开源成熟的 jar 包或者框架实现。成熟的开源 jar 包或者框架，有大量完备的测试以及广泛的用户来确保质量。 如果实在需要自己造小轮子，请使用单元测试来确保质量

5.大量 if else 语句

大量的 if-else 语句，具体情况具体分析。但是大部分都可以用卫语句提前返回结果。避免大量嵌套。

例如左边的写法可以改为右边的写法

像下图这种情况可以用 Stream Lambda 来进行简化

优化后

6.多余的代码判断

有些时候可能会写出一些不必要的冗余判断

7.大量的代码细节让阅读者增加心智负担

我们应该封装一部分代码细节，暴露出代码的主流程，优化后

8.繁琐的代码逻辑

像上图这种情形，我们其实可以使用一句 Stream 语句就可以描述出来。

9.数据库中是与否可以直接用 tinyInt 映射，不要用字符串来映射

这样会造成布尔字段取出时，还需要跟字符串 1 或者 0 进行比对，这是很尴尬的设计。

10.异常捕获之后不做任何处理

我们捕获异常之后一般都需要使用 log 来记录错误情形，如果什么都不做，就很可能丢失错误信息，并且使代码排查过程更加困难。

11.使用 Map 填充数据

使用 Java 是静态语言，使用 Map 填充数据，反而失去了静态语言带来了代码检查以及 IDEA 识别字段引用的功能。

12.混乱的常量

请不用将项目中所有的常量一股脑的放到一个类中。

可以使用像这种静态类的方式，分门别类地放入不同的常量

13.请使用驼峰命名

14.变量的定义请在系统内保持一致，比如 1 在系统内表示是。 请勿有时表示是，有时表示否。

有时候用 1 表示肯定，有时候用 0 表示肯定，有时候用 Y 表示肯定。

15.奇葩的代码脑回路

在外层方法判断一遍，在内层方法又进行一遍一样的判断

16.常量随意的命名格式

常量的命名请使用大写加下划线的格式

17.嵌套的 Switch 语句

就一种 case 了 完全没有必要使用 switch 语句

18.使用魔法值

例如项目中大量使用了“jpg”的字符串魔法值，使用魔法值使得我们无法统一找到代码的引用处。在重构的时候难免会有疏漏。

19.单个方法代码超过 80 行

如果单个方法的代码行超过 80 行，意味你的代码缺乏封装和可读性。例如这种一大坨的代码。

public static void initColumnField(GenTableColumn column, GenTable table)
{
    String dataType = getDbType(column.getColumnType());
    String columnName = column.getColumnName();
    column.setTableId(table.getTableId());
    column.setCreateBy(table.getCreateBy());
    // 设置 java 字段名
    column.setJavaField(StringUtils.toCamelCase(columnName));
    // 设置默认类型
    column.setJavaType(GenConstants.TYPE_STRING);
    column.setQueryType(GenConstants.QUERY_EQ);

    if (arraysContains(GenConstants.COLUMNTYPE_STR, dataType) || arraysContains(GenConstants.COLUMNTYPE_TEXT, dataType))
    {
        // 字符串长度超过 500 设置为文本域
        Integer columnLength = getColumnLength(column.getColumnType());
        String htmlType = columnLength >= 500 || arraysContains(GenConstants.COLUMNTYPE_TEXT, dataType) ? GenConstants.HTML_TEXTAREA : GenConstants.HTML_INPUT;
        column.setHtmlType(htmlType);
    }
    else if (arraysContains(GenConstants.COLUMNTYPE_TIME, dataType))
    {
        column.setJavaType(GenConstants.TYPE_DATE);
        column.setHtmlType(GenConstants.HTML_DATETIME);
    }
    else if (arraysContains(GenConstants.COLUMNTYPE_NUMBER, dataType))
    {
        column.setHtmlType(GenConstants.HTML_INPUT);

        // 如果是浮点型 统一用 BigDecimal
        String[] str = StringUtils.split(StringUtils.substringBetween(column.getColumnType(), "(", ")"), ",");
        if (str != null && str.length == 2 && Integer.parseInt(str[1]) > 0)
        {
            column.setJavaType(GenConstants.TYPE_BIGDECIMAL);
        }
        // 如果是整形
        else if (str != null && str.length == 1 && Integer.parseInt(str[0]) <= 10)
        {
            column.setJavaType(GenConstants.TYPE_INTEGER);
        }
        // 长整形
        else
        {
            column.setJavaType(GenConstants.TYPE_LONG);
        }
    }

    // 插入字段（默认所有字段都需要插入）
    column.setIsInsert(GenConstants.REQUIRE);

    // 编辑字段
    if (!arraysContains(GenConstants.COLUMNNAME_NOT_EDIT, columnName) && !column.isPk())
    {
        column.setIsEdit(GenConstants.REQUIRE);
    }
    // 列表字段
    if (!arraysContains(GenConstants.COLUMNNAME_NOT_LIST, columnName) && !column.isPk())
    {
        column.setIsList(GenConstants.REQUIRE);
    }
    // 查询字段
    if (!arraysContains(GenConstants.COLUMNNAME_NOT_QUERY, columnName) && !column.isPk())
    {
        column.setIsQuery(GenConstants.REQUIRE);
    }

    // 查询字段类型
    if (StringUtils.endsWithIgnoreCase(columnName, "name"))
    {
        column.setQueryType(GenConstants.QUERY_LIKE);
    }
    // 状态字段设置单选框
    if (StringUtils.endsWithIgnoreCase(columnName, "status"))
    {
        column.setHtmlType(GenConstants.HTML_RADIO);
    }
    // 类型&性别字段设置下拉框
    else if (StringUtils.endsWithIgnoreCase(columnName, "type")
            || StringUtils.endsWithIgnoreCase(columnName, "sex"))
    {
        column.setHtmlType(GenConstants.HTML_SELECT);
    }
    // 图片字段设置图片上传控件
    else if (StringUtils.endsWithIgnoreCase(columnName, "image"))
    {
        column.setHtmlType(GenConstants.HTML_IMAGE_UPLOAD);
    }
    // 文件字段设置文件上传控件
    else if (StringUtils.endsWithIgnoreCase(columnName, "file"))
    {
        column.setHtmlType(GenConstants.HTML_FILE_UPLOAD);
    }
    // 内容字段设置富文本控件
    else if (StringUtils.endsWithIgnoreCase(columnName, "content"))
    {
        column.setHtmlType(GenConstants.HTML_EDITOR);
    }
}

20.避免使用反逻辑

像图中!(x == 0) 可以直接改成x != 0 即可

还有以下这种冗余代码。

21.复杂的判断使用有意义的变量来替代

比如上图的判断我们可以用一个变量 isLongField 来替代 提高代码的可读性。

22.常量没有使用 final 来修饰

如果没有使用 final 来修饰的话，就有可能在代码中被修改。

23.字符编码直接用字符串表示

字符编码，JDK 中都有常量可以直接表示，我们可以直接使用

24.多余的方法修饰符

Java 中 interface 类，方法默认都是 Public 的，没必要再加上 public 修饰符

25.不必要的 ToString

26.多余的变量声明

如果变量声明之后没有做任何处理，请直接通过 return 返回，不要多声明一个变量

27.使用语义不清晰的方法

例如 String 的 indexOf 方法 我们完全可以使用 contains 方法来替代，使代码的语义更一目了然。

28.毫无必要的包装语句 unboxing 和 boxing

Integer.valueOf

返回的本身就是 int, 没有必要再调用 intValue 方法

29.使用+=进行在循环中字符串拼接

+=会造出临时的字符串，我们应该使用 StringBuilder 在循环中拼接字符串

以上就是总结的关于项目中的一些坏实践，请大家务必使用。有其他坏实践，恳请大家继续补充。

---

鄙人在业余时间弄了一个全栈项目 Agileboot ，初衷是想做一个代码规范，项目结构良好，可供大学生或者入门 3 年内的开发者参考使用的项目。

后端地址： https://github.com/valarchie/AgileBoot-Back-End
鄙人能力水平有限，如果项目中发现不足或者错误，恳请指正。欢迎 PR 。一起构建一个规范完善的后端项目。

前端地址： https://github.com/valarchie/AgileBoot-Front-End

鄙人前端小白，关于前端项目的规范以及优化仅作了力所能及的部分，还有很多优化空间。哪位前端大佬有兴趣一起帮忙规范和优化吗？

演示地址 www.agileboot.vip

欢迎加入全栈技术交流群：1398880

[Hccake]

第 n 个基于 ruoyi 修改的项目了= =

[Aloento]

文章虽好，但是推广

[ngduncent]

@Aloento 我也没有盈利目的 的确是希望有人一起来构建一个规范的全栈项目 所以放了链接...
很过分吗....

[TWorldIsNButThis]

我也来 review 一下
为什么用 StringUtils 判断 isNull ，而且这里转成-1 是为了？
这里不是直接用 name 查出来库内值，判断库内值为 null 或者上传值 id 等于库内值的 id 就行了？

2202 年了还用在 Date ，这么喜欢受虐嘛

这个 foreach + if else add + removeAll null 把我看笑了

另外优化后的 allOnlineUsers 为什么要 collect 出来，就算要起个名字直接用 stream 不就好了，collect 就会执行至少一次迭代

这么喜欢用 map 建议用 php


看不下去了，原来的代码写的啥玩意。。

[ngduncent]

@TWorldIsNButThis
嗯 这还是 3 万星的项目...



“优化后的 allOnlineUsers 为什么要 collect 出来，就算要起个名字直接用 stream 不就好了，collect 就会执行至少一次迭代”

嗯 感谢你的指正。晚上 fix~

[hsfzxjy]

建议改为《从 Java 开源项目中总结出的 Java 项目的常见坏实践》

[ngduncent]

@hsfzxjy 好的 哈哈哈哈 不然其他语言会躺枪

[netabare]

感觉和开源与否没什么关系，因为看下来很多都是新人容易犯的错误，在私有仓库里面估计也是泛滥成灾。而且开源项目也不是只有 java 吧。

话说过来一大半的截图都有下划线，也就是说很多问题都是可以靠 intellisence 和 lint 来解决的吧。感觉把 idea 的提示用好，并且装上一个够用的 linter ，基本上就能避免大部分这样类型的错误顺便学到一些良好的编码习惯。

op 的题目会让人认为是和开源项目的贡献、组织管理或者维护有关的事情，多少有点名不副实了。

[Bingchunmoli]

@TWorldIsNButThis 说实话无脑用 Date 的报错真的少，降低心智，LocalDateTime jackson 需要配置，springboot 接受参数也要配置。Date 一把梭， 虽然我也很认同 Date 在不同时区造成的各种问题，但是大部分公司的项目，至少小公司的项目不需要考虑时区，甚至程序员自身都不懂时区，Date 也不是那么的不行

[Bingchunmoli]

大部分时认同的，不过 steam 的看法就有些出入，如果他是针对新手的项目 一定程度刻意不用 steam 我觉得是 OK 的，我也经历过项目中一段一段的 stream 看不懂在干啥(现在也许不该用过去的眼光看，这个我持谨慎态度， 毕竟 JDK1.8 是老特性了)。

[misaka19000]

不错

[ngduncent]

@netabare sorry 题目的确有歧义 我的锅
主要想表达的是高星开源项目（ 3W star ）中一些坏的代码实践。或许称不上坏实践，而是纯粹新手错误。

[ngduncent]

@Bingchunmoli 嗯 一开始我也是抵触 stream 的 后面用完感觉还是挺香的

[IvanLi127]

第 24 条，虽然默认是 public ，但是经常切换不同语言的开发者不一定记得，这时候减少心智负担显式声明，还算可以理解吧。。。。毕竟你搞全栈的😏

[adoal]

getDatePoor 实在蚌埠柱了

[FrankHB]

你这些开不开源没直接关系吧。
给你补个强相关的：
0. 瞎写 copyright notice/瞎用许可证。

[WilliamYang]

第 26 不完全同意，我看过谷歌开源项目的代码，有时即时同个值，也会重新定义一个更符合当前语义的变量，让别人阅读代码更好理解，否则，按你所说就会出现 return getxxxXXx() + getYyyyyYY() + magicNum 很长的语句了

[Macolor21]

一眼 ruoyi

[Jtyczc]

19. 确实有这么多字段，怎么优化呢

[ngduncent]

@FrankHB 好的 感谢指正

[ngduncent]

@WilliamYang 嗯 你这个情况是对的
我主要是想说 这种简单的情况
Money m = getMoney();
return m;

[ngduncent]

@Jtyczc 第 19 条问题并不出在 这么多字段
而是像 JavaType , htmlType, queryType 这几个字段的设值, 在整段代码中出现很多处。
如何对几个字段依情况设值， 可以抽取出小段代码封装。
代码会明了很多

[wu67]

那个 0 1 字符串的经常见到. 之前有个同事就喜欢这种, 啥都是 0 1, 都没见过 boolean 型, 问就说可能后续会加其他状态, 特么的我前端绑在开关上的变量, 开和关还有啥其他状态...

[ngduncent]

@wu67 是的 像文中提到的方法 检查名字在系统内是否唯一 结果 要么唯一 要么不唯一
用 Boolean 型 简单很多

[dengji85]

mark ， 我下载过很多这种开源脚手架学习过，那时就有类似的感受，这代码和我写的也差不多，因为工作以后一直在搞一个内部系统，代码就是这种风格，没机会去学习到比较规范的实现。不过 ruoyi 已经是为数不多可用的，因为确实能跑起来，可以做生产；因为有很多假开源的，文档要钱，限制并发数，连支持事务都要上商业版的

[zhanlanhuizhang]

代码质量和开源没有什么关系。不然开源怎么兴起。怎么说了，你用了它的代码，你觉得不好，请提交你的代码合并请求。你只批评，没有意思。

[dengji85]

@wu67 我是前端后端一把梭的也有这个习惯，状态用字符，确实考虑的是扩展，你那个关和开是一种特殊情况，有时用习惯了就这样

[ngduncent]

@zhanlanhuizhang 如果是少部分代码不好 我会选择提 PR 。（我也不想浪费时间，自己起一套项目）
绝大部分代码都不好。 我选择自己改一套。

[ngduncent]

@zhanlanhuizhang 再者 我的 PR 作者也不一定认同。 大家各自理念不一样。

[senninha]

29. 1.5 开始编译器会自动转换成 stringbuilder

[ngduncent]

@senninha 感谢你的提醒！ got it.

[senninha]

@ngduncent 刚看了一下 29 在 1.8 下的情况，虽然用了 stringbuilder 拼接，但是还是会产生大量的 stringbuilder 临时对象，我想当然了。

[ngduncent]

@senninha 嗯， 因为 IDEA 是有提示不要 loop 中+=。 所以我有点怀疑。


所以也自己试验了一遍，100w 次 +=耗时 4 秒多 stringBuilder.append 耗时 1 秒
public class TestString {

public static void main(String[] args) {
testStringBuilder();
}

private static final int loop = 1000000;

public static void testPlus() {
System.out.println(">>> testPlus() <<<");

long start = System.currentTimeMillis();

for (int i = 0; i < loop; i++) {
String str = "";
for (int j = 0; j < 100; j++) {
str += j;
}
}
long end = System.currentTimeMillis();
long cost = end - start;

System.out.println("{str+=} cost=" + cost);
}

public static void testStringBuilder() {
long start = System.currentTimeMillis();

for (int i = 0; i < loop; i++) {
StringBuilder str = new StringBuilder();

for (int j = 0; j < 100; j++) {
str.append(j);
}
}

long end = System.currentTimeMillis();
long cost = end - start;

System.out.println("{stringBuilder.append} cost=" + cost);
}
}

[wizzer]

https://demo.budwk.com

[loopinfor]

文不对题，就拿了个开源的噱头。
说是开源项目的坏实践，但是截图连来源于哪个项目都没讲。
这样的案例，是不是淘宝买几份毕业设计源码，然后自己上传 github ，就可以拿学生练手项目的代码作为开源坏实践来讲了？

[ngduncent]

@wizzer nice

[ngduncent]

@loopinfor 标题起的不好 我的锅
说了哪个项目 可能要喷我引战
不说哪个项目 又说我连个项目都不说

我好难啊
btw. 这是三万星项目的代码~ 的确很多大学生拿去毕设~~

[opentrade]

差评

[ngduncent]

@loopinfor 我要是直接说了哪个项目~ 引流效果更好哦~~ 但是没必要~

[ngduncent]

@opentrade 感谢你的评价 （ doge ）

[Rache1]

5 、不知道 Stream 内部会不会有相干优化， 但是这样看起来就是把原本遍历一次的，变成了遍历多次了，不过从可读性来讲，肯定是 Stream 的方式更易懂。

13.请使用驼峰命名，方法可以用驼峰，但是路由还是得中划线连接符比较好。

26.多余的变量声明，这样多写一行有个好处就是在开发阶段，调试时可以调整返回值和看到返回值吧，不过 IDEA 新版好像在 return 的跳出前显示最终值了。

---

看上去这些都是些业务代码，算是比较常见的问题了。

[ngduncent]

@Rache1
13. 嗯 是的 路由不同驼峰，感谢指正。
26. 现在编程环境都比较智能 应该都支持显示 return 值了

是比较常见的问题， 但是太多大学生学习这些代码了...

[YepTen]

你这是把阿里的<Java 开发手册>又复制了一份吗.

[Rache1]

@ngduncent 其实这类内容，看起来更像是最佳实践的方向；对于学生而言，他们的实践知识较为薄弱，对于一些像 Stream 之类的东西，都不一定有教，一些简单冗余的代码更易于他们的理解，就就像上面举例的滥用三元表达式。在他们学习的时候，或许只关注到了（ expr1 ? expr2 : expr3 ），expr1 成立时返回 expr2 ，否则返回 expr3 ，或许在他们看来，这也是河里的，还有魔法值那一块 😆

随着深入的了解后他们也会意识到这些问题。

如果想要学习一些好的源码那应该去看一些如 Spring 。这种业务形的项目，更多的在于了解让他们怎么做出来了，后续还有优化的空间不是。

[ngduncent]

@YepTen 对对对 你说对了

[lzrainchen]

大致浏览了一下这些问题，我觉得就两大类问题：
第一类：代码组织类问题，比如常量全部写到一块没有按逻辑区分，大量逻辑堆叠在一起难以阅读、这些都是代码组织能力问题，包括设计模式的问题都是大量的练习中学到的，我认为设计模式其实就是解决了大量代码如何清晰地组织的问题，这些问题无法短时间提升只能靠写得多慢慢提升
第二类：代码技巧类问题，比如滥用三元运算符、使用 lambda 表达式、冗余代码、使用 final 修饰、多余方法修饰、字符编码使用字符串等问题 这些问题大都 IDEA 都会有提示的（黄色波浪线）使用 alt+enter 都会给出相应提示，这个应该很多人都知道，我觉得我从入行开始非常早就开始使用 IDEA （那时候 IDEA 还不流行大部分人还是使用 Eclipse 、MyEclipse ）我觉得从 IDEA 的提示我学会了写代码，还有一个好处就是不但学会了写代码还知道这个错误的问题用英文怎么描述（不会有人用中文的 IDEA 吧 不会吧 不会吧... :)）这顺便就把第一个英文差的问题给解决了...
总结：写代码不需要特别高的智商，普通人就非常足够，无非就是细心细心细心。这些工具告诉给你的错误，框架报给你异常、不断地出错，不断地思考，不断地解决这些错误就够了

[Zizpop]

2 中 图一的最佳实践是什么? (没用 java)

[ngduncent]

@lzrainchen 是的。 细心+对待代码的态度。
有的人觉得代码能跑就行 剩下不重要。 有的人觉得代码丑 就很难受。

[ngduncent]

@Zizpop
这个也没啥最佳实践， 而是图中那个例子没必要用三元运算符
大部分语言 应该都可以把 return 1>0 ？ true:fasle; 直接改成 return 1>0; 吧？
你用的是哪个语言？

[dyxiaodong2022]

三元运算符那个例子我还真干过，函数返回 true/false ，还加了个 ? true : false ，hhh

[Zizpop]

@ngduncent 嗯, 我没表达清楚, 我想的是「是不是可以把 result 去掉」

[Georgedoe]

第一次见代码质量这么差的开源项目

[tairan2006]

没用过质量这么差的开源项目。。

这种质量还不如我自己写

[msg7086]

@dyxiaodong2022
动态语言是会有这样写的，比如 return obj ? true : false 这样。
一般要简化的话会简化成 return !!obj 。

[ngduncent]

@msg7086 原来是这样。 感谢指点。

[hai046]

@tairan2006 我感觉人家开源了， 并且有人 star 就有他的价值，感觉对自己没用不看他就是，没必要贬低他，

[zhengfan2016]

用 IDEA 等 JB 家的可以解决至少一半坏实践，例如三元写 true 和 false 等等 IDE 基本都会用红色下划线标注出来，提示你有更正确的写法