8种代码臭味

09-10-23 banq
千里之行,始于足下,千里之堤,毁于蚁穴,做好设计的第一步就是写好你的代码,博文8 Signs your code sucks总结了代码中第一个感觉的臭味,让你能够于细微之处发现软件的质量问题。

1.方法内代码超过一个电脑屏幕:

一个方法只应该执行特定的任务,一个方法不应该包含一些这样的逻辑,例如判断用户名字段包哈巴的数据是否有效,是否存在等。如果一个方法代码大得超过一个屏幕,那么这是表明它做了太多的事情,需要切分。

2.你在重用变量:

除非你工作于嵌入式领域,否则内存是便宜的,不要做内存的守财奴,要注重可维护性。

3.你直接访问request/session :

这和具体应用服务器环境绑定,难于测试,所有应用数据应该直接解耦Session/request,保存到Bean中,通过 bean的 getters 和 setters方法, 创建使用者访问数据的合约,这将大大帮助代码的可维护性,个人补充,不要把类中的Collection字段直接通过Collection getCollection来暴露给外界,通过方法封装对Collection的操作。

4.你需要使用注解来解释代码如何使用:

代码应该自己能够解释它如何使用,易于可读,如果你发现你自己都需要注解专门解释如何使用,那么就要重构你的代码。这里注解不是指javadoc等必要文档。

5.一个exception系列错误没有返回最原始的错误:

你不应该吃掉exception错误,在catch一个exception时,要打印出其出错tack trace. 如果不知道错误来源,如何纠正错误呢?

6.你的代码是一堆泥球:

代码粘在一起,机会没有分离分层,代码应该是模块化,这样易于维护和重用。 MVC是关注用户View视图界面发生的事情,控制器是关注程序流程和数据的校验,而处理业务逻辑是领域模型的事情,只有模型可以和数据库访问直接进行交互。

7.难于单元测试

如果你发现Bug,那么些一段新的代码片段,它会花去你一些时间,但是这样代码就能处理更加复杂的事情了。

3
Hqiu
2009-10-23 10:55
3.你直接访问request/session

关于这一点,其实很多框架已经封装处理过,就是用一个ActionBeanContext对request、Session进行处理,然后将其实例注入ActionServlet内,这样就可以达到间接操纵request等目的。比如在最近参考的Stripes里面就有这种行为:

public class ActionBeanContext {

private HttpServletRequest request;

private HttpServletResponse response;

private ServletContext servletContext;

private String eventName;

private ValidationErrors validationErrors;

................

public class CalculatorActionBean implements ActionBean {

private ActionBeanContext context;

@Validate(maxvalue = 20)

private double numberone;

private double numberTwo;

private double result;

private String captchaResponse;

public CalculatorActionBean() {

}

public ActionBeanContext getContext() {

return context;

}

public void setContext(ActionBeanContext context) {

this.context = context;

}

在mock测试的时候手动替换自己的ActionBeanContext 即可。

“4.你需要使用注解来解释代码如何使用:”

改成“如果你需要用注释解释你的代码”是否可行?

“7.难于单元测试”

改成“坚持写单元测试”是否可行?

banq
2009-10-23 11:11
多谢楼上指正。

关于代码味道和设计模式关系见访谈:

GoF设计模式三作者15年后再谈模式

hantsy
2009-10-28 13:36
Hey, stripes guys.

Do not miss my

stripes tutorials.

[该贴被hantsy于2009-11-13 10:06修改过]

lendo
2009-10-29 16:23
1.方法内代码超过一个电脑屏幕:

一个方法只应该执行特定的任务,一个方法不应该包含一些这样的逻辑,例如判断用户名字段包哈巴的数据是否有效,是否存在等。如果一个方法代码大得超过一个屏幕,那么这是表明它做了太多的事情,需要切分。

----------------------------------------------------------------------------------------------------

有时候没办法,修改的代价太高,特别是针对遗留代码,最近维护了一个类。

5400多行,一个方法内有214个case,也想过重构,但开销过大。

BTW:个人觉得大到软件架构,小到几个类的设计,中庸一点比较好,既不充血也不贫血。

lendo
2009-10-29 16:24
1.方法内代码超过一个电脑屏幕:

一个方法只应该执行特定的任务,一个方法不应该包含一些这样的逻辑,例如判断用户名字段包哈巴的数据是否有效,是否存在等。如果一个方法代码大得超过一个屏幕,那么这是表明它做了太多的事情,需要切分。

----------------------------------------------------------------------------------------------------

有时候没办法,修改的代价太高,特别是针对遗留代码,最近维护了一个类。

5400多行,一个方法内有214个case,也想过重构,但开销过大。

BTW:个人觉得大到软件架构,小到几个类的设计,中庸一点比较好,既不充血也不贫血。

Hqiu
2009-10-30 11:17
to hantsy:

<br>

在使用stripes时,发现资料很少,于是本人也想着自己写一本关于Stripes的书,刚写了一部分时,不经意发现了先生已经写了一本, 而且大部分内容都有涉及,于是我就在此书的基础上进一步修改完善了一下,也增加了一部分内容。本来打算写好了以后发给先生指点一下,但是大概快要完成时,本人又发现一本关于Stripes的书,此书是stripes官方推荐的书,名字是Stripes..And Java web development is fun again,我发现此书讲得非常全面深刻,令自己汗颜,故不敢再写下去,还望见量!

<br>

随便说一下,banq先生有空也该完善一下jdon,还有很多细节不完善,修改的时候根本换不了行,还得输br。

[该贴被Hqiu于2009-10-30 12:24修改过]

azvf
2011-02-22 10:29
个人补充,不要把类中的Collection字段直接通过Collection getCollection来暴露给外界,通过方法封装对Collection的操作。

=================================

业务中需要getCollection<T>() ,那要如何封装。为什么要不暴露,这其中有什么道道么

猜你喜欢