实践中的重构08_集合判空时的可读性

代码的可读性应该还是着重强调的，毕竟，代码是给人读的。如

if(null==a)

这样的代码，看的出是从c惯用法继承下来，试着读一下，当null不为a，则如何如何。至少我读起来有点别扭。我还是喜欢这样的形式，当a不为null，则如何如何。
当然，更厉害的是经常看到如下的片段

if(list.size()==0)
if(list.size()<1)

ok,size==0这个我还可以想的通为什么这么写，但是<1这种写法真的让人无语。

if(list.isEmpty())

这样当然更进一步。为什么呢，因为使用size是因为我们知道该类的内部实现，判断时涉及了类的实现，而isEmpty只是语义上判断一个列表是否为空，不涉及实现细节。
还有一种挺常见的，好的做法是集合类引用对象永远不用判null。

    if(list!=null && !list.isEmpty()){  
      // do something.  
    }  

我们可以把对list的判null和判空抽出来，变成

    if(!CollectionUtil.isEmpty(list)){  
      // do something.  
        }

更进一步。

    if(CollectionUtil.isNotEmpty(list)){  
      // do something.  
    }  

问题在于，null和empty在特定的上下文中真的是一样的吗。

评论

20 楼 mercyblitz 2010-12-21  

zhang_xzhi_xjtu 写道

mercyblitz 写道

zhang_xzhi_xjtu 写道

代码的可读性应该还是着重强调的。

如

if(null==a)

这样的代码，至少我读起来有点别扭。

当然，更厉害的是

if(list.size()<1)

我知道有些人喜欢写成

if(list.size()==0)

ok,这个我还可以想的通为什么这么写，但是<1这种写法真的让人无语。

if(list.isEmpty())

这样当然更进一步。

毕竟，代码是给人读的。

好吧，其实一般都是这么用的

if(list!=null && !list.isEmpty()){
//xixihaha
}

我们可以把对list的判null和判空抽出来，变成

if(!CollectionUtil.isEmpty(list)){
//xixihaha
}

问题在于，null和empty在特定的上下文中真的是一样的吗。

list.size()==0 没有什么可读性差的。只是个人风格地问题。


为什么不这么写呢？

if(CollectionUtil.isNotEmpty(list))

这样写的可读性更好。
问题在于，null和empty在特定的上下文中真的是一样的吗。

不能完全肯定和否定，很多业务需要区分null和empty的！

一般null通过断言判断，在参数传递时，但是CollectionUtil类为了容错性，必须判断null到做empty!

19 楼 zhang_xzhi_xjtu 2010-12-20  

mercyblitz 写道

zhang_xzhi_xjtu 写道

代码的可读性应该还是着重强调的。

如

if(null==a)

这样的代码，至少我读起来有点别扭。

当然，更厉害的是

if(list.size()<1)

我知道有些人喜欢写成

if(list.size()==0)

ok,这个我还可以想的通为什么这么写，但是<1这种写法真的让人无语。

if(list.isEmpty())

这样当然更进一步。

毕竟，代码是给人读的。

好吧，其实一般都是这么用的

if(list!=null && !list.isEmpty()){
//xixihaha
}

我们可以把对list的判null和判空抽出来，变成

if(!CollectionUtil.isEmpty(list)){
//xixihaha
}

问题在于，null和empty在特定的上下文中真的是一样的吗。

list.size()==0 没有什么可读性差的。只是个人风格地问题。


为什么不这么写呢？

if(CollectionUtil.isNotEmpty(list))

这样写的可读性更好。
问题在于，null和empty在特定的上下文中真的是一样的吗。

18 楼 mercyblitz 2010-12-20  

zhang_xzhi_xjtu 写道

代码的可读性应该还是着重强调的。

如

if(null==a)

这样的代码，至少我读起来有点别扭。

当然，更厉害的是

if(list.size()<1)

我知道有些人喜欢写成

if(list.size()==0)

ok,这个我还可以想的通为什么这么写，但是<1这种写法真的让人无语。

if(list.isEmpty())

这样当然更进一步。

毕竟，代码是给人读的。

好吧，其实一般都是这么用的

if(list!=null && !list.isEmpty()){
//xixihaha
}

我们可以把对list的判null和判空抽出来，变成

if(!CollectionUtil.isEmpty(list)){
//xixihaha
}

问题在于，null和empty在特定的上下文中真的是一样的吗。

list.size()==0 没有什么可读性差的。只是个人风格地问题。


为什么不这么写呢？

if(CollectionUtil.isNotEmpty(list))

17 楼 zhang_xzhi_xjtu 2010-12-20  

一切行为都要受到实际情况的限制。

16 楼 smallhand 2010-12-20  

tuti 写道

zhang_xzhi_xjtu 写道

我是完全同意集合为空的情况下不返回null而是返回一个空集合这个惯例的。
但是有以下两种情况会有纠结的地方。
1 底层代码无法改动，加入一个间接层会提高别的程序员的使用成本。
2 大型项目中，一个人所能改动的代码是有范围限制的，对于自己没有改动权限的代码问题，最多提提建议。所以这个重构也只能局限在自己的系统中。

既然知道什么是对的，那就要去做。
说说都知道，但都不去做，那和不知道又有什么区别呢。
软件开发，本来就是个协作游戏，需要相互沟通和反馈，这在大型项目中，尤其重要。

极限编程（XP）主要的价值观包括：沟通，简单，反馈，勇气，尊重。
换言之，如果缺少“沟通，反馈，勇气，尊重”，那么“简单”也是做不到的。

这个返回null不就是个例子吗？

我同意你的看法。但是就我们公司来说，即使你有想法要动，公司领导也不同意。因为他们大多要的是稳定，能用就行。我们的jdk还是1.4的，浏览器IE7有的功能不能用。但是就是说以后再动。也是无奈啊

15 楼 yuan 2010-12-20  

:) 是呀，既然是一起做事，代码还是集体所有好些。面子问题一边去。

说来说去，agile对程序员的素质是有要求的，选人、组建团队很关键，招聘是第一关。招聘搞好了，麻烦事会少很多。扯远了……

14 楼 tuti 2010-12-19  

zhang_xzhi_xjtu 写道

我是完全同意集合为空的情况下不返回null而是返回一个空集合这个惯例的。
但是有以下两种情况会有纠结的地方。
1 底层代码无法改动，加入一个间接层会提高别的程序员的使用成本。
2 大型项目中，一个人所能改动的代码是有范围限制的，对于自己没有改动权限的代码问题，最多提提建议。所以这个重构也只能局限在自己的系统中。

既然知道什么是对的，那就要去做。
说说都知道，但都不去做，那和不知道又有什么区别呢。
软件开发，本来就是个协作游戏，需要相互沟通和反馈，这在大型项目中，尤其重要。

极限编程（XP）主要的价值观包括：沟通，简单，反馈，勇气，尊重。
换言之，如果缺少“沟通，反馈，勇气，尊重”，那么“简单”也是做不到的。

这个返回null不就是个例子吗？

13 楼 zhang_xzhi_xjtu 2010-12-19  

yuan 写道

tuti 写道

list 就不应该为null

对，除非是第3方库某个本该返回集合的方法返回了个null才需要这么处理（我认为那也是这个库的作者不专业，而且即使是第3方库也可以自己修改）。如果是自己写的方法，从语义上说，这个方法的返回值如果是一个集合，那在该方法运行结果是这个集合为空的情况下，怎么也不应该返回null。1是影响可读性，2是其它每个调用该方法的地方还要处理这个null。返回null是自己给自己找麻烦。

LZ在另一帖中 写道

get底层的服务，别人未必想的那么周全，如果它的注释中，没有声明一定不返回null的话，我觉得判一下null是比较安全的做法。

如果给自己一个“集合为空的情况下不返回null而是返回一个空集合”的原则，就不会有这个问题。

而且即使遇到别人的代码在返回空集合时用null替代，这时候是把别人的代码改简单些好呢？还是把自己的代码改复杂来适应别人的代码好？

这样的话，lz在http://www.iteye.com/topic/841196中的代码最后可以变成这样：

	private List<User> getUserList2(int size) {

		List<User> userList = new ArrayList<User>();

		userList.addAll(getUserListFromSource1(size - userList.size()));
		
		if (userList.size() >= size) return userList;

		userList.addAll(getUserListFromSource2(size - userList.size()));
		
		if (userList.size() >= size) return userList;

		userList.addAll(getUserListFromSource3(size - userList.size()));

		return userList;
	}

而且这代码还有改进的余地。但是没有上下文（对getUserList的调用；还有getUserListFromSourcex的定义），我只能写成这样。

我是完全同意集合为空的情况下不返回null而是返回一个空集合这个惯例的。
但是有以下两种情况会有纠结的地方。
1 底层代码无法改动，加入一个间接层会提高别的程序员的使用成本。
2 大型项目中，一个人所能改动的代码是有范围限制的，对于自己没有改动权限的代码问题，最多提提建议。所以这个重构也只能局限在自己的系统中。

12 楼 zhang_xzhi_xjtu 2010-12-19  

这个方法我记得在代码大全2里面有介绍过，也是一种好的方法。
我的平铺直叙也是这个意思，尽量使代码语言贴近自然语言。

11 楼 baby8117628 2010-12-18  

学习了 这个方法不错哦 翻译法...

10 楼 yeshucheng 2010-12-18  

检验下自己，也是一个不合格的程序员，反省学习

9 楼 yuan 2010-12-18  

tuti 写道

楼上给解释一下

其实蛮简单的，和LZ所说的平铺直叙同一个意思，举个最近写的代码的例子：

首先是描述最直接，最表面的需求：

定义 类：节点服务器
  定义 方法： 启动节点服务器
    方法实现： 
    加载配置文件()
    IPQAM开始广播()
    连接上分发服务器()
    开始监听客户端连接和请求()
    每5秒检查并清除一次死掉的终端连接()
    记录系统启动日志()

然后会发现隐藏在更低一层的需求，发现需要定义加载配置文件，应该由一个专门配置系统的类来做：

定义 类：配置
   定义 方法： 加载
   方法实现：
   xxxxxxxx
   ......

然后是IPQAM的广播：

定义 类：IPQAM
   定义 方法： 广播
   方法实现：
   遍历所有频道
      频道.发送广播()
   遍历结束

然后又发现隐藏在IPQAM中的更底层的需求，需要定义一个频道要处理频道自身的逻辑：

定义 类：频道
   定义 方法： 发送广播
   方法实现：
   UDPSocket.new.connect(ip, port).send('boradcast message', 0)

只要把以上人类语言翻译成相应的代码即可。

8 楼 tuti 2010-12-18  

yuan 写道

有一种很简单实用的方法可以写出较高质量的代码（包括强可读性，绝对的DRY，足够的可扩展）：假设式脚本编写法（scripting by assumption）或称一厢情愿式编程法（programming by wishful thinking）。

楼上给解释一下

7 楼 yuan 2010-12-18  

lz在另一帖中说到“平铺直叙”，我想起有一种很简单实用的方法可以写出较高质量的代码（包括强可读性，绝对的DRY，足够的可扩展）：假设式脚本编写法（scripting by assumption）或称一厢情愿式编程法（programming by wishful thinking）。

6 楼 yuan 2010-12-18  

tuti 写道

list 就不应该为null

对，除非是第3方库某个本该返回集合的方法返回了个null才需要这么处理（我认为那也是这个库的作者不专业，而且即使是第3方库也可以自己修改）。如果是自己写的方法，从语义上说，这个方法的返回值如果是一个集合，那在该方法运行结果是这个集合为空的情况下，怎么也不应该返回null。1是影响可读性，2是其它每个调用该方法的地方还要处理这个null。返回null是自己给自己找麻烦。

LZ在另一帖中 写道

get底层的服务，别人未必想的那么周全，如果它的注释中，没有声明一定不返回null的话，我觉得判一下null是比较安全的做法。

如果给自己一个“集合为空的情况下不返回null而是返回一个空集合”的原则，就不会有这个问题。

而且即使遇到别人的代码在返回空集合时用null替代，这时候是把别人的代码改简单些好呢？还是把自己的代码改复杂来适应别人的代码好？

这样的话，lz在http://www.iteye.com/topic/841196中的代码最后可以变成这样：

	private List<User> getUserList2(int size) {

		List<User> userList = new ArrayList<User>();

		userList.addAll(getUserListFromSource1(size - userList.size()));
		
		if (userList.size() >= size) return userList;

		userList.addAll(getUserListFromSource2(size - userList.size()));
		
		if (userList.size() >= size) return userList;

		userList.addAll(getUserListFromSource3(size - userList.size()));

		return userList;
	}

而且这代码还有改进的余地。但是没有上下文（对getUserList的调用；还有getUserListFromSourcex的定义），我只能写成这样。

5 楼 tuti 2010-12-18  

list 就不应该为null

4 楼 aoliwen521 2010-11-30  

list.size()<1
这么些我觉得没什么可读性差的。只是少了null的判断。

3 楼 mercyblitz 2010-11-28  

大多数时候关心是集合中 有元素,没有的话,可以不考虑!

2 楼 zhang_xzhi_xjtu 2010-11-28  

IcyFenix 写道

null == a是c/c++年代遗留下来的写法，那时候IDE不会检查条件语句里面的赋值，为了避免写成if(a=null)，现在这种写法在Java中是没什么必要了。

java里面这种写法比较有意义的是和常量比较的时候，if(a.equals(CONST_STRING))就没有if(CONST_STRING.equals(a))来的好。后面不会产生空指针异常。如果实在不喜欢判空，建议使用NullObject模式。

同理，下面的list，考虑防御性原则，前面没有判空的话，也经常会写if(list==null || list.isempty())

遗留的写法的原因我是知道的，所以想着大家在java世界中不要这么搞了。

if(CONST_STRING.equals(a))这个是有一定的道理。

IcyFenix 写道

同理，下面的list，考虑防御性原则，前面没有判空的话，也经常会写if(list==null || list.isempty())

这个当然你是对的。但是我的题目是可读性，所以list和null的判断就没有写出来。
我补上了对判null的想法。

1 楼 IcyFenix 2010-11-28  

null == a是c/c++年代遗留下来的写法，那时候IDE不会检查条件语句里面的赋值，为了避免写成if(a=null)，现在这种写法在Java中是没什么必要了。

java里面这种写法比较有意义的是和常量比较的时候，if(a.equals(CONST_STRING))就没有if(CONST_STRING.equals(a))来的好。后面不会产生空指针异常。如果实在不喜欢判空，建议使用NullObject模式。

同理，下面的list，考虑防御性原则，前面没有判空的话，也经常会写if(list==null || list.isempty())