由一段代码发表一点想法

今天在公司发现了一段很怪异的代码，为此还跟公司员工争执了一下，但由于自身是新员工，我只有无奈的屈服了，心里确实不爽，在这里发表一下自己的看法。先看一下代码，代码已经我已经简化了，只有两个类，一个是action层，另一个是Service层，具体如下：
Action：

package com.yf.test;

import java.util.List;

public class Action {
	public void excute(){
		List list=null;
		list=Service.doSomething();
		if(list.isEmpty()){
			/*
			 *此处省略其他操作
			 */
		}
	}
}

Service

package com.yf.test;

import java.util.ArrayList;
import java.util.List;

public class Service {
	public static List doSomething() {
		List list = new ArrayList();
		if(1==2){
			/*
			 * 此处省略对list的其他操作
			 *  list=.....
			 */			
		}
		return list;
	}
}

当我看到if(list.isEmpty()){时，发现潜在产生空指针异常的可能，然后就给同事说了一下，这里应该先判断List是否为null，然后他让我看Service层，说Service不会返回null，我看了一下，确实不会返回为null，但是关于这段代码，我觉得写的实在太烂，为什么烂，主要有一下几个原因：
1、在Service层，每次调用doSomething方法时，都实例化一个List，虚拟机都会在堆中为这个list开辟内存，这无疑实在浪费内存和虚拟机的，而且这个list只有在if条件成立时，才需要，如果if不成立，虚拟机还得在方法调用结束后，回收这块内存，这难道不是没事找事吗？？
2、在Action层，action不对返回的list做非null判断，这也是一种很恶心的做法，首先，根据面向对象的封装性，Service层中的实现对Action而言，应该是不可见的，Action层应该对其返回值的可能情况做判断，即list!=null必须在Action做，如果后续Service层单独抽出，以API提供Jar包的形式，即我们无法知道里面的具体细节，这时，Action层还得做非空判断。因此，本人觉得，这段代码应该做如下重构
Action：

package com.yf.test;

import java.util.List;

public class Action {
	public void excute(){
		List list=null;
		list=Service.doSomething();
		if(list!=null&&list.isEmpty()){
			/*
			 *此处省略其他操作
			 */
		}
	}
}

Service

package com.yf.test;

import java.util.ArrayList;
import java.util.List;

public class Service {
	public static List doSomething() {
		List list=null;
		if(1==2){
			list= new ArrayList();
			/*
			 * 此处省略对list的其他操作
			 *  list=.....
			 */			
		}
		return list;
	}
}

或许并不是每个人都认同我这种做法，不过我个人觉得这样比较合理，软件设计的时候要讲究层次，各层应该干得事情，就应该在所在层做好，而不是有其下层来保证，这种强依赖下层保证是一种很恶心的做法，如果后续下层代码逻辑变更，还得去上层看看对其的影响，这就很无耻了！！！

评论

18 楼 林晓盼 2012-10-14  

话说那个if条件咋看也是有点问题啊？if（1==2）？？？

17 楼 michael8335 2012-10-14  

young_suse 写道

楼主的担心也很合理，每次在方法开头用List list = new ArrayList();确实可能造成不必要的开销。
但从代码规范的角度来讲，声明为集合（List或数组）返回类型的函数，通常不要返回null，宁可返回size为0的空集合。
要达到既减少内存开销，又规范代码，应该使用List list = Collections.emptyList()来取代List list=null;并且按照kidneyball说的，用javadoc来说明，当条件不满足时，返回size为0的List，而不会返回null。

由于我编码时间不长，所以估计是我自身理解确实有问题，不过通过这篇文章，获得了一些知识，看来大家的意见还是倾向于不返回null，那就按照大家的来吧，反正我只是就这段代码发表一下自己的看法，谢谢大家的意见，后期会买一本《Effective Java》，在设计方面继续积累

16 楼 michael8335 2012-10-14  

StrongZhu 写道

除非写代码的人能说服我，不然此处的代码一定是

 final List<String> someList = Service.doSomething();  
 if(CollectionUtils.isEmpty(someList)){  
    .
    .
    .
}

我觉得你没有看懂我要表达的意思

15 楼 young_suse 2012-10-14  

楼主的担心也很合理，每次在方法开头用List list = new ArrayList();确实可能造成不必要的开销。
但从代码规范的角度来讲，声明为集合（List或数组）返回类型的函数，通常不要返回null，宁可返回size为0的空集合。
要达到既减少内存开销，又规范代码，应该使用List list = Collections.emptyList()来取代List list=null;并且按照kidneyball说的，用javadoc来说明，当条件不满足时，返回size为0的List，而不会返回null。

14 楼 StrongZhu 2012-10-14  

除非写代码的人能说服我，不然此处的代码一定是

 final List<String> someList = Service.doSomething();  
 if(CollectionUtils.isEmpty(someList)){  
    .
    .
    .
}

13 楼 jinzty 2012-10-14  

个人感觉还是避免返回null的好，你这是if判断，额外判断什么的不说，while的时候会使代码的可读性变差吧

12 楼 kidneyball 2012-10-14  

forcer521 写道

我支持你兄弟，null判断增加了代码，但是却增强了代码的健壮性。
很多时候，团队的规范并不是每个人都能彻底执行的。

“所有对象类型的返回值都要做null判断”也是团队规范，也有人会不遵守。方法是写来多次重复使用的，从团队角度来看，你觉得要求写底层方法的人遵守某个规范容易，还是要求使用这个方法的人每次使用时都遵守某个规范容易？

11 楼 flashing 2012-10-14  

sqtds 写道

opnmzxcvb 写道

kidneyball 写道

1. 容器类型的底层方法应该尽量返回空集，避免返回null。从而避免顶层方法做大量的null判断。具体可参考《Effective Java》。

2. 一个以容器为返回类型的方法，返回空集通常是不常见的异常分支，为一个不常见的分支考虑类创建和垃圾回收层面上的性能问题而导致调用者做大量的额外NP判断并不值得。这类优化应该在有profiling数据支持的情况下再做。

3. “具体细节不可见”的问题，写点Javadoc就能解决。调用者需要参考接口文档知道返回值的具体情况，是很正常的事情。后续维护时要保证接口协议不变，也是很正常的事情。

4. 某些团队都会形成共识，只要是返回容器类型的方法就不返回null。只要大家都遵守这个协议，不会出问题而且可以免去大量无谓的NP判断。博主作为一个新人，应该先了解团队中的开发规范，而不是和现有规则较劲。

说的对，  

看看effective java

yeah，我也想说 effective java

10 楼 Chris_bing 2012-10-14  

michael8335 写道

yawei 写道

你说的是理论上的东西， 而代码呈现的是实际开发中的最好实现。 server层的实现很合理， list先建立就保证了无论条件如何， 总会返回一个list。

action不检查null是因为，在这种模式下， null意味着重大错误， NPE是可以接受的结果。

这种设计逻辑很多地方都能见到，例如搜索引擎。

我在文中说过了，这里只是我自己的想法，我觉得对Action层来说，它就应该做自己的事情，比如做空判断，为什么要下层来保证，如果Service层对Action不可见后，那是不是还是要加上非空判断呢？？？

不可见后也是可以看到Javadoc的,或者说团队规范就是这样的,大家一致就可以了

9 楼 forcer521 2012-10-14  

jorneyR 写道

在职场中，不要轻易去表现出质疑别人的东西，就算那东西再差，自己不赞同，也尽量不要表现出来，人心难测，除非你有决策权的时候。

很有经验啊，呵呵。
我们程序员很多时候就是缺乏这种思维能力。
很多时候，做的对并不能得到大家的认可的。

8 楼 forcer521 2012-10-14  

我支持你兄弟，null判断增加了代码，但是却增强了代码的健壮性。
很多时候，团队的规范并不是每个人都能彻底执行的。

7 楼 sqtds 2012-10-14  

opnmzxcvb 写道

kidneyball 写道

1. 容器类型的底层方法应该尽量返回空集，避免返回null。从而避免顶层方法做大量的null判断。具体可参考《Effective Java》。

2. 一个以容器为返回类型的方法，返回空集通常是不常见的异常分支，为一个不常见的分支考虑类创建和垃圾回收层面上的性能问题而导致调用者做大量的额外NP判断并不值得。这类优化应该在有profiling数据支持的情况下再做。

3. “具体细节不可见”的问题，写点Javadoc就能解决。调用者需要参考接口文档知道返回值的具体情况，是很正常的事情。后续维护时要保证接口协议不变，也是很正常的事情。

4. 某些团队都会形成共识，只要是返回容器类型的方法就不返回null。只要大家都遵守这个协议，不会出问题而且可以免去大量无谓的NP判断。博主作为一个新人，应该先了解团队中的开发规范，而不是和现有规则较劲。

说的对，  

看看effective java

6 楼 opnmzxcvb 2012-10-14  

kidneyball 写道

1. 容器类型的底层方法应该尽量返回空集，避免返回null。从而避免顶层方法做大量的null判断。具体可参考《Effective Java》。

2. 一个以容器为返回类型的方法，返回空集通常是不常见的异常分支，为一个不常见的分支考虑类创建和垃圾回收层面上的性能问题而导致调用者做大量的额外NP判断并不值得。这类优化应该在有profiling数据支持的情况下再做。

3. “具体细节不可见”的问题，写点Javadoc就能解决。调用者需要参考接口文档知道返回值的具体情况，是很正常的事情。后续维护时要保证接口协议不变，也是很正常的事情。

4. 某些团队都会形成共识，只要是返回容器类型的方法就不返回null。只要大家都遵守这个协议，不会出问题而且可以免去大量无谓的NP判断。博主作为一个新人，应该先了解团队中的开发规范，而不是和现有规则较劲。

说的对，  

5 楼 kidneyball 2012-10-14  

1. 容器类型的底层方法应该尽量返回空集，避免返回null。从而避免顶层方法做大量的null判断。具体可参考《Effective Java》。

2. 一个以容器为返回类型的方法，返回空集通常是不常见的异常分支，为一个不常见的分支考虑类创建和垃圾回收层面上的性能问题而导致调用者做大量的额外NP判断并不值得。这类优化应该在有profiling数据支持的情况下再做。

3. “具体细节不可见”的问题，写点Javadoc就能解决。调用者需要参考接口文档知道返回值的具体情况，是很正常的事情。后续维护时要保证接口协议不变，也是很正常的事情。

4. 某些团队都会形成共识，只要是返回容器类型的方法就不返回null。只要大家都遵守这个协议，不会出问题而且可以免去大量无谓的NP判断。博主作为一个新人，应该先了解团队中的开发规范，而不是和现有规则较劲。

4 楼 jorneyR 2012-10-14  

在职场中，不要轻易去表现出质疑别人的东西，就算那东西再差，自己不赞同，也尽量不要表现出来，人心难测，除非你有决策权的时候。

3 楼 michael8335 2012-10-14  

yawei 写道

你说的是理论上的东西， 而代码呈现的是实际开发中的最好实现。 server层的实现很合理， list先建立就保证了无论条件如何， 总会返回一个list。

action不检查null是因为，在这种模式下， null意味着重大错误， NPE是可以接受的结果。

这种设计逻辑很多地方都能见到，例如搜索引擎。

我在文中说过了，这里只是我自己的想法，我觉得对Action层来说，它就应该做自己的事情，比如做空判断，为什么要下层来保证，如果Service层对Action不可见后，那是不是还是要加上非空判断呢？？？

2 楼 yawei 2012-10-14  

你说的是理论上的东西， 而代码呈现的是实际开发中的最好实现。 server层的实现很合理， list先建立就保证了无论条件如何， 总会返回一个list。

action不检查null是因为，在这种模式下， null意味着重大错误， NPE是可以接受的结果。

这种设计逻辑很多地方都能见到，例如搜索引擎。

1 楼 kjj 2012-10-14  

     
我觉得你说的很对