improve bitter code: 没有行为的封装

备注: 示例中的代码并不是真实代码的完全拷贝

常见的对象

javabean这种java类是非常常见的,它有一些属性和相应的get/set方法。 我们还会看到pojo,vo这些概念,还有我们使用的一种特殊对象cbo,在外观上都是这种形式。

在项目的代码中,我们经常大量这种对象,它有私有变量,对每个私有变量都提供了get/set方法,除此之外,没有其他方法。 这种情况下,通常只需写好变量,然后用工具生成相应的访问器方法。

在最近修改一处代码的时候,就看到下面的逻辑,并且这段代码在多个文件出现过。

PrepaidInput prepaid = ...// load from request

String start = prepaid.getStartTime();
String end = prepaid.getEndTime();

if(StringUtils.isNotEmpty(start)){
String newstart = start.replaceAll("-", "");
if(newstart.length() > 8){
newstart = newstart.substring(0, 9);
prepaid.setStartTime(newstart);
}
}

// 继续处理结束时间end

封装还需要行为

上面的代码是很常见的处理方式,对象只是传值的作用。给变量封装到方法里边,提供了get/set方法。 本质上来说,就像换了个马甲,和直接暴露数据差不多。类似的代码出现很多,先看下面简单点得例子:

if("1".equals(subs.getStatus())){
//TODO
}

没有行为,顶多算是基于对象的编程。 从行为及职责考虑,这里暴露了状态值1,因为调用方必须理解这个值的作用。 封装,除了封装数据,还得封装出行为的样子来。如:

if(subs.isActive()){
//TODO
}

对于最前面的代码,修改方式有很多,有一种方式就是在set里边, 组装对象的时候调用set方法顺便把格式给处理了。
没有规定说get.set都会对应一个私有变量,也没有规定get.set要成对出现,如果这是在内部使用的变量,为什么要暴露出来?
同样除了get/set也没有规定不能提供其他的方法。

void setStartTime(String startTime){
String newstart = startTime;
if(StringUtils.isNotEmpty(startTime)){
String newstart = startTime.replaceAll("-", "");
if(newstart.length() > 8){
newstart = newstart.substring(0, 8);
}
}
this.startTime = newstart;
}

现实情况

可惜的是,系统中重要的数据载体是cbo,由工具生成的,类似于vo。所以cbo没什么像样的行为。
对于开发java web的同学,习惯cbo,mvc这样的流水线作业,使用面向对象编程,对封装行为并不习惯。
引用郑大的话:我们需要更好的封装,通常的做法是封装出行为。行为从哪来,从实际需求来。
所以,还是得在平时工作中有意识、有针对性的实践才行呀!

"improve bitter code"系列文章:

命名风格培训草稿

我记得郑大有次开课的时候,提到命名这个话题。说有次他跟一个前辈说:命名真的太难了。 前辈说:你终于开窍了。哪天咋们心里也有这个意识,也算是开窍了。(昨晚忘记说这个事了,补充)

编码风格与编码规范

风格是带主观色彩的,有个人爱好的区别。
规范是双刃剑,限制个人风格的随意发挥,让整体有个统一的风格。
规范是一些风格的最佳实践的集合,但仍然是不断进步的。

学习规范,应该从产生背景出发,理解规范背后希望解决的问题,才能更好理解规范,减少抵制情绪,或许还能改进规范。

破窗效应与童子军军规

破窗效应: 一个房子如果窗户破了,没有人去修补,隔不久,其它的窗户也会莫名其妙地被人打破; 一面墙,如果出现一些涂鸦没有被清洗掉,很快的,墙上就布满了乱七八糟、不堪入目的东西; 一个很干净的地方,人们不好意思丢垃圾,但是一旦地上有垃圾出现之后,人就会毫不犹疑地抛,丝毫不觉羞愧。 (选择汽车为例子也可以)
童子军军规: 当你离开一个地方的时候,要让它比你来的时候更整洁干净。

遵守童子军军规,警惕破窗效应。避免对代码大范围进行修改
对于老代码,总会出现新老风格冲突的情况,优化代码应该控制修改的范围和脚步,采用结对或评审的方式规避风险。
加强代码评审, 一个人出错的几率是5%,两个人在同个地方犯错的几率才0.25%。

文件名规则

遵循约定大于配置原则,像java文件名本身是和类名有关系的,对于页面文件和配置文件,同样在命名上要存在相关性。思考下面的问题:
如果你知道其中一个文件,如何找到相关的文件

包名规则

包名规则本身就是有一套指导方针的:
以公司网址进行打头,如com.gmcc
通常会附带项目名,如com.gmcc.boss
项目大的话,还会继续模块化,如com.gmcc.boss.product
模块下面在继续分功能,从分层的角度,可以有com.gmcc.boss.product.action、service、dao、bean之类的包

对于通常的web应用,模块化划分也可以从菜单入手进行分解,至少可以作为参考。模块如果很大,可以再拆分子模块

常用命名法

Camel命名 – java标准命名法
匈牙利命名 – MFC那套,变量名都带类型,如今有IDE智能提示,显得过时了。而且有货不对板的风险。
pascal命名 – 和Camel就第一个单词首字母的区别
下划线命名 – 在c代码、javascript、ruby等脚本中比较常见

类名的选择

主要从业务逻辑,框架要求,模式设计方面考虑:
业务逻辑中按分层命名规范即可 – Action Controller Service Call Dao Business Bean等都是常用的后缀
框架基础类,参考接口命名 – Filter Servlet Comparable *Utils *s
使用设计模式时参考常见命名 – Adapter Chain Visitor Factory Builder

虽然整个代码结构使用分层方式,但还是有地方可以考虑设计模式的,切勿错失良机。
设计模式方面的内容参考四人帮的书籍,关于jdk的设计模式参考酷壳的文章

对于业务系统,最好准备一份业务术语词典,用来指导相关业务功能的命名

方法命名

评价标准:一句话功能描述
通常选择动词加名词的组合,常用的动词并不多。 命名方式可能有所不同,取决于代码的关注点。(默念,参考评价标准)如:
getSubsByCertId – 通过证件号获取用户信息,有强调证件号的意思
getSubs – 获取用户信息,关注结果,可能支持多种方式

变量命名

对于临时变量,通常使用i,j,k,c,b等就可以了。不应该作为其他作用存在。
对于标记标识,选择done,found,success,ok,fail等,切勿使用flag, 评价标准(填空后读起来很顺畅):假如(找到了)…
对于状态,整理一些常用的常量以备使用,如:READY,SUCCESS,FAILED
对于常量,望文生义+注释(注释在这里显得很有必要),修改checkstyle的魔鬼数字时,是学习命名的好机会
对于普通变量,我们常常会带上类型如subsattrList,我觉得subsattrs这种单复数形式会更好。不过这是个习惯问题,不是什么大问题。 变量命名经验要多学习优秀开源代码,会有很多好处。有些变量名是由IDE通过方法名自动生成的,方法名恰当的时候,生成的变量名也是不错的。(总结)

命名技巧都差不多,我经常采用办法:在心里默念需要的功能,转换成方法名/变量名。反复默念几遍命名,看看是否能够像句子一样读出来。

注释

代码不能代替注释,但优先考虑代码的可读性,例如使用方法抽取等手段。
可读性的标准是代码可以像文章一样读出来,保证方法名和注释相符合,忌讳牛头不对马嘴。 坚持这种态度的话,代码质量一定会有所提高。

注释关注类注释和方法注释,方法内注释用于代码难以表达的地方(如某种特殊情况)。
对于外围接口(包括前后台调用),在方法注释中进行详细说明。

需求单号应该适当添加到代码中,我觉得类注释可能是比较合适的地方。
修改bug的代码通常比较烂(现网的是这样),可以标注一下BUG单号等信息。
提交到版本库的日志要清晰,重点关注什么原因修改,修改哪些功能点等,争取让每次提交都有据可依(每次日志都是不一样的)

improve bitter code: 判空的处理

备注: 示例中的代码并不是真实代码的完全拷贝

预料之外

预料之外的情况,防御性编程经常会让整洁划一的代码变得混乱。 我们经常会担心各种各样的空值,例如null不行,换成空字符串,集合为空或取不到值,怎么办? 字符串都是空格,要不要trim一下? 往一个很深的属性中取值,判空的逻辑太多了?

if(Objects.isNotEmpty(subs)){
List<Attr> subsAttrs = subs.getAttrs();
if(Objects.isNotEmpty(subsAttrs)){
for(Attr attr : subsAttrs){
if(attr != null){
//do something with attr
}
}
}
}

诸如此类的情况每天都会出现,的确是很麻烦的。 更多材料可以查看一下Google Guava关于避免null的描述

用assert减少麻烦

有一种叫assert的技巧,用来保证参数必须满足一定的先决条件,不满足则无法继续。 这作为和客户端代码之间的协议,即使有异常情况,也应该在客户端先处理好。 当然,assert通常用来防御不正常的调用。以spring这种ioc框架为例,通过get,set无法保证某些必填的值都设对了, 所以在调用业务接口的时候再assert一把。看下面的代码。

方式1
public Map queryWokrfResult(String recnum){
Assert.isNotNull(recnum, "业务流水号不能为空!");
// query with recnum
}
方式2
public Map queryWokrfResult(String recnum){
if(StringUtils.isEmpty(recnum)){
throw new ReceptionException("业务流水号不能为空!");
}
// query with recnum
}

我是这么看待的,方式1表示你可以传空,但我会做检测,不会让你得逞。相当于防御性编程。 方式2,你就不应该传空,你的参数不符合接口规范。 虽然assert的实现或许就是判断的简单封装,但这里这么不能简单理解。 assert与其说是编程技巧,不如说是编程模式。

对于java来说,本身是支持assert的,但这个特性一般没人使用。大多数人选择用异常来封装,例如spring assert,commons-lang validate等都提供了自己的一套assert api。需要注意的时候,assert里边的条件应该都是非常快的,不要把业务逻辑混淆进去。

封装,封装,由底层处理

实际上,提供一些工具类(如常用的判空操作和带默认值操作)可以减轻痛苦, 还可以根据具体应用在底层框架上处理,在提高容错性的同时,让代码更优雅。

在项目的老代码中,我们会看到这样的判空处理。

// example 1
MapVo vo = new MapVo();
if(StringUtils.isNotEmpty(name)){
vo.setAttribute("name", name);
}
if(StringUtils.isNotEmpty(subsid)){
vo.setAttribute("subsid", subsid);
}
// ... other conditions

DBUtils.queryByVo("queryXXX", vo);

为什么这么写,是因为框架的sql生成配置只提供了null的配置方式,而不支持常见的空字符串,无内容的字符串等常见模式。 为此,通过增加特性,像ibatis那样支持isEmpty模式的配置,就可以省略大量显式判断。 修改后的代码如下:

// example 2
MapVo vo = new MapVo();
vo.setAttribute("name", name);
vo.setAttribute("subsid", subsid);
// ... other conditions

DBUtils.queryByVo("queryXXX", vo);

再看,例如老页面jsp里边这样的逻辑也很多(嵌入java代码)

<% Subscriber subs = request.getAttribute("subscriber"); %>
<% if(subs != null) {
out.print("<span>" + subs.getServnumber() + "</span>");
<% } %>

对于这种代码有很好的方式,如el表达式,又或者velocity等模板解决方案,都具有很好的容错性。

再来看看其他语言框架的解决模式,如javascript框架jquery,对错误保护就很好,对不存在的东西,提供了默认行为: 视而不见。 通过基础框架提供的保障,避免一些琐碎的工作,代码可以更关注于逻辑处理。

空对象模式

空对象模式就是用一个特殊的对象代替空对象,这个对象和正常对象有着同样的接口,但调用的时候会表现出异常处理逻辑。wiki上有关于这个模式的各种编程语言的描述。 通过这种模式,把异常处理逻辑封装起来,对于客户端代码来说,调用方式和正常情况是一样的。

例如,对于数据查询接口返回空的列表而不是null,使用一些空集合对象。

java.util.Collections#emptyList()
java.util.Collections#emptyMap()
java.util.Collections#emptySet()

对于返回列表的接口,使用空列表对象,很多判空的逻辑就没有必要存在了。 又例如,返回不可修改或线程安全的特殊列表,也可以看成是这种模式的变化

Arrays#asList()

java.util.Collections#unmodifiableList()
java.util.Collections#unmodifiableMap()
java.util.Collections#unmodifiableSet()

java.util.Collections#synchronizedList()
java.util.Collections#synchronizedMap()
java.util.Collections#synchronizedSet()

总结

对接口进行约定,约束参数和返回值,可以简化判空的处理。

在框架层面进行统一封装,可以专注于业务逻辑。

"improve bitter code"系列文章:

improve bitter code: 多掌握一门语言

备注: 示例中的代码并不是真实代码的完全拷贝

语言是有区别的

语言是有’好坏’之分的,在不同的应用背景下,有些语言就是比其他一些语言更为合适! 语言是有它的设计目的,有些是为了效率(c),有些是为了快乐编程(ruby),有些则是了并发做准备(erlang)。 学习不同类型的语言也有助于锻炼编程思维,像c,java,ruby,javascript,erlang都教会了我许多东西。

来到这边之后,主要工作语言是java,但并不是说其他语言没有了用武之地,我一直在寻找使用其他语言的机会。 下面举些例子,来说明一下。由于我以前大量使用ruby语言,的确我也是特别喜欢。 所以例子以ruby为主,但并不是说其他语言没法做到,或许有更好的做法也不一定。

临时生成xml文件

前几天维护那边需要我提供一个xml文件,它提供了一组txt数据:每半个小时的登陆及操作统计。 我需要生成一个xml,我不熟悉excel操作(花了几分钟尝试了一下),所以我选择使用脚本来生成。 脚本只花了一分钟,比我预想得还要顺利。

open('a.xml', 'w') do |f|
open('a.txt', 'r') do |ff|
ff.readlines.each_with_index do |l, i|
dl, cz = l.split(' ')
f << "<data><seq>#{i+1}</seq><czvalue>#{cz}</czvalue><dlvalue>#{dl}</dlvalue></data>"
end
end
end

对于这种一次性的文本处理,java真是太繁琐了,没有任何优势可言。

定位数据问题

曾经有次上版本之后发现一个菜单树不能使用。通过代码定位应该是菜单出现循环依赖了(菜单在表中是父子关系的)。 同事也把生产的数据拿过来了。数据格式类似下面的,第一列是菜单id,第二列是父菜单id,还有其他的数据列。

ITEM1 ITEM2 OTHERS...
ITEM2 ITEM3 OTHERS...
ITEM4 ITEM2 OTHERS...
ITEM5 ITEM3 OTHERS...

问题已经变成:父子是否出现循环。当时我用了几分钟写了一段脚本来找到出现循环的菜单项,最后发现是其他项目组增加的菜单有问题。

传数据的偷懒方法

以前还是老邮箱的时候,由于网络隔离,需要把测试相关的东西通过邮箱发到测试环境去。 我每次都是这么弄的:通过网页登录邮箱,新建邮件,把已经打包加密好的文件作为附件添加,保存为草稿,退出。 次数多了,真的很无聊,有时候一天要弄好几次。

我知道有种测试方式叫自动化测试,所以我用watir(一个用ruby写的自动化测试框架)模拟了整个过程。后来我把这个脚本调用集成到菜单右键上, 只需要把需要上传的文件选中,再触发一个右键菜单。打包加密,上传文件就搞定了。那时候的感觉就是,世界清净了。

实现codediff应用

有些框架工具有非常高的生产效率,ruby on rails就是其中一个。在项目中进行codediff使用的工具,主要是通过这个框架开发的。 说真的,这么一个东西,用java实现也不是什么难事。但对我来说,1k行的ruby代码量,sql一行也没有写,维护上要省心得多。

只要能够对项目有意义,谁会关心我是用什么来实现的呢?

部署应用

这是个cmo比较熟悉的领域。大多数情况下,我们会选择bash等shell工具。 不过我在开发codediff工具的时候,我想在本地直接就把应用部署到开发环境上去。

windows的bat很烂,shell等有比较多限制,所以我又选择了脚本语言。 使用perl,ruby等脚本来做管理脚本并不少见,我这里选择一个好几年前写的脚本作为例子:

cmd = Proc.new do |shell|
shell.cd("/opt/xxx")
print "current directory:",shell.pwd.stdout
print shell.sh("/opt/getupdate.sh").stdout
shell.exit
end

Net::SSH.start('192.168.1.32', :username => 'root', :password=>'psword') do |session|
shell = session.shell.sync
cmd.call(shell)
end

上面的例子比较简单,现实的例子是本地打包,通过ftp上传,解压发布,重启服务器,每个步骤都带交互功能。 脚本语言和普通shell相比,具有完整语言和跨平台的优势,用来做系统管理是一个不错的选择。

多掌握一门语言

整篇内容没有涉及编程技巧上的东西,但这却是我最想表达的内容。 在这里,我用Martin Fowler中国行的时候,图灵社区对他的采访内容来结束, 里边有关于学习编程语言建议的内容。

图灵社区:您曾经建议过程序员应该每年学习一门编程语言。

M:这实际上是Pragmatic Programmers提出的建议,是他们出版的书中的一条建议,我本人很赞同。我认为应该有意地学习一些新的语言,特别是那些和你所熟知的语言的运作方式相差甚远的语言,所以不要在相似的语言上浪费时间。如果你是一位Java程序员,那么C#对于你来说就太过熟悉了,你需要学习一些很不同的语言,比如说Lisp,或者Clojure;如果你是一位Lisp程序员,你也不需要学Clojure,因为那就是另一种Lisp而已。总之你应该尝试一些很不同的语言。

"improve bitter code"系列文章:

improve bitter code: 对付魔鬼数字

备注: 示例中的代码并不是真实代码的完全拷贝

道高一尺,魔高一丈

魔鬼数字在项目中大量出现,是代码可读性维护性变差的重要推手。 为了控制事态恶化,项目中加入findbugs,checkstyle等静态检查工具,试图让人自觉修复魔鬼数字这类头疼的问题。

最近在review N项目的代码时,发现有新童鞋对修复checkstyle的问题热情高涨,有些修改的确是有益的, 但是对魔鬼数字的修改就显得不是正路了。大家先看看:

private static final int INT_2 = 2;
private Static final int INT_80 = 80;
private Static final int INT_1000 = 1000;
private Static final int INT_60000 = 60000;

这种情况以前也出现过,那个时候刚推广checkstyle,结果有人定义了NumberUtil类,里边就有ONE, TWO…等变量, 一直延伸到好几十。这让我非常纠结,劝说了很多次,结果这个类最终还是扎下根来了。我对此深恶痛绝。

今天再次看到这种方式,真是道高一尺魔高一丈呀。我又要发牢骚了。

认清工具的本质

这种做法完全就是为了对付checkstyle,而不是对付那对问题代码。工具本来是想提醒程序员,避免写出难以维护的代码。 结果我们更聪明,把代码藏起来,眼不见为净。这样其实是把数字和变量名直接关联起来了, 你能想象一个叫INT_2的变量值确是1么? 如果在不同含义的逻辑里边使用这样一个魔鬼变量, 一旦值发生变化,就会顺带把不应该修改的地方也改动了。

如果我们真的是为了提高自己的编码水平,提高项目的代码质量,就应该认真的对待这些魔鬼数字。 对于魔鬼数字,应该有一个良好的命名,对于经常出现的,还应该找个合适的地方加以管理。接下来以上面的例子继续分析。

解决魔鬼数字

要解决魔鬼数字,首先是认清魔鬼数字代表的意义,给它起一个合适的名字。

查看源代码,可以发现,做了下面的代码调整:

  1. 2是某个值在列表中的位置索引,换成XXX_IDX
  2. 80是http的默认端口,换成DEFAULT_HTTP_PROT
  3. 1000是用来把秒换成毫秒的,可以用MILLISECONDS_PER_SECOND
  4. 60000是默认超时时间来的,换成DEFAULT_TIMEOUT

同样,经常会看到魔鬼字符串,”0”,”1”这样的值出现代码里边,也应该 把给他们起一个好的名字,例如作为执行结果,有SUCCESS、FAILED等。

更进一步

需要考虑的还有常量定义的地方,另外考虑使用方法封装来处理魔鬼数字的逻辑。 这样可以隐藏一些细节,把作用域限制在很小的地方。 例如下面经常出现的魔鬼字符串,就可以考虑把整个判断逻辑封装到user类里边去, 而不只是简单的定义出ACTIVE这样一个常量。

// 原有的代码
if("1".equals(user.getStatus())){
// ...
}

// 改造后,"1"作为user的常量使用于isActive方法
if(user.isActive()){
// ...
}

不过话说回来,解决魔鬼数字、字符串之类等’小枝小节’,花费的精力不小。 但不要忽视这些细节,这样可以让自己不断提高对代码的认识,也便于其他人维护你的代码。

对付魔鬼数字,还是不要走捷径的好!

"improve bitter code"系列文章: