由一段代码发表一点想法

今天在公司发现了一段很怪异的代码，为此还跟公司员工争执了一下，但由于自身是新员工，我只有无奈的屈服了，心里确实不爽，在这里发表一下自己的看法。先看一下代码，代码已经我已经简化了，只有两个类，一个是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;
	}
}

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

评论

58 楼 jayming 2012-10-17  

我同意楼主的观点，空指针很多时候并不是错误，我们不应该让程序在没有错误的情况下抛出空指针异常，参考hibernate返回list的方法，@return a result object returned by the action, or <code>null</code>

57 楼 黯然小伙 2012-10-16  

貌似很多讨论。我的想法是：
项目中最好不要返回null(经历过很多血的教训)，尤其是对list和map。
我项目中的解决方法是：

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

仅供参考，个人愚见。

56 楼 奔三的小生 2012-10-16  

返回一个空对象也不应该返回null，不要对内存太较真

55 楼 maimode 2012-10-16  

kidneyball 写道

谈谈返回null值的问题。在2009年，“null”的发明人就发表了一次演讲，说引入null这个概念是他犯下的一个巨大错误，几十年来造成了软件行业超过十亿美元的损失。（http://www.infoq.com/presentations/Null-References-The-Billion-Dollar-Mistake-Tony-Hoare）。因此我个人的倾向是，除非不得已，不要返回null。

以博主的场景为例，如果我在团队里规定，返回容器类型的方法不能返回null。那么对使用者来说，这个方法总共就三种合法情况：返回非空容器，返回空容器，抛出异常。如果发现返回值为null，那就说明是这个方法本身出错了，使用者可以直接反馈给编写者排错。

反之，如果团队里规定，“返回容器类型的方法里可能返回null”，那么对使用者来说，这个方法有以下几种可能情况：
1. 返回非空容器
2. 返回空容器
3. 抛出异常
4. 返回null，其意义相当于空容器
5. 返回null，其意义相当于抛异常
6. 返回null，有特殊的业务含义
7. 编写者没处理好，不小心返回了null（毕竟空指针是最常见的运行期错误，没有之一）。

使用者拿到一个这样的方法，就必须要在4、5、6、7之间做出选择。如果编写者没有写Javadoc，必须要看具体实现才能确认4和5，要加上一定推理才能确认6。无论选4,5,6，都无法排除可能是7。

说的很透彻！其实楼主和他的同事只是奉行的风格不同罢了，没有说那种风格一定好，但是能确定的是，团队保持一致的风格非常重要。

54 楼 greatghoul 2012-10-16  

kidneyball 写道

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

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

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

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

对，很多流行框架也是这么做的。

53 楼 skzr.org 2012-10-16  

教人家编程会让其痛苦一辈子

52 楼 jinnianshilongnian 2012-10-16  

mfkvfn 写道

jinnianshilongnian 写道

1、在Service层，每次调用doSomething方法时，都实例化一个List，虚拟机都会在堆中为这个list开辟内存，这无疑实在浪费内存和虚拟机的，而且这个list只有在if条件成立时，才需要，如果if不成立，虚拟机还得在方法调用结束后，回收这块内存，这难道不是没事找事吗？？

如果服务层 调用 持久层框架（如hibernate） 即使是空 也是返回一个集合的；
                 自己写业务  如果为null  可以返回Collections.EMPTY_LIST(单例 且不可变的) 
建议：不要过早优化 先保证代码可读和可维护

“为空时返回不一个不可变的Collections.EMPTY_LIST单例”这个是不可以的。
一个方法有可能会返回可变的ArrayList有可能会返回不可变的单例，这是不实现的。

比如

    private static final List<String> XXX = new ArrayList<String>(0);

    private List<String> getList(int a) {
        if (a == 1) {
            return new ArrayList<String>();
        } else {
            return XXX;
        }
    }

    private void b() {
        List<String> b = getList(1);
        b.add("abc");
    }

在编译时不会出错。运行时可能会出错。

这些要看自己怎么处理了，基本没有定式；
如查询 可以返回Collections.EMPTY_LIST
如返回的数据需要修改  建议再做一个List保存处理结果 而不是在之前的List中修改
看自己编码习惯了

51 楼 mfkvfn 2012-10-16  

jinnianshilongnian 写道

1、在Service层，每次调用doSomething方法时，都实例化一个List，虚拟机都会在堆中为这个list开辟内存，这无疑实在浪费内存和虚拟机的，而且这个list只有在if条件成立时，才需要，如果if不成立，虚拟机还得在方法调用结束后，回收这块内存，这难道不是没事找事吗？？

如果服务层 调用 持久层框架（如hibernate） 即使是空 也是返回一个集合的；
                 自己写业务  如果为null  可以返回Collections.EMPTY_LIST(单例 且不可变的) 
建议：不要过早优化 先保证代码可读和可维护

“为空时返回不一个不可变的Collections.EMPTY_LIST单例”这个是不可以的。
一个方法有可能会返回可变的ArrayList有可能会返回不可变的单例，这是不实现的。

比如

    private static final List<String> XXX = new ArrayList<String>(0);

    private List<String> getList(int a) {
        if (a == 1) {
            return new ArrayList<String>();
        } else {
            return XXX;
        }
    }

    private void b() {
        List<String> b = getList(1);
        b.add("abc");
    }

在编译时不会出错。运行时可能会出错。

50 楼 lianglaiyang 2012-10-16  

michael8335 写道

finallygo 写道

第一点我支持博主的,因为本来定义的接口就是可能多种实现的,虽然有的人说什么团队规范,但是一个团队规范不能保证所有人都认真的执行,其次还增加了一种潜规则在里面,所以,我采用不信任编程,另外第二个,我觉得还是尽量不要返回null

谢谢了，其实我写这个只是表达一下我的想法，同事想看看大家的看法，不过根据大家的评论来看，有两点是明显的，1、上层要做非空判断；2、下层不应该返回null。感谢大家的评论，从中学习不少。

从人与人相处来说，作为一个新人，楼主应该先向你同事公开道歉，以后遇到意见不合时，就要虚心一点。这个是最重要的，不然如果不汲取这次教训，很难混下去

49 楼 weng 2012-10-16  

yawei 写道

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

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

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

胡扯

48 楼 alfusen 2012-10-16  

学习了,学习了！

47 楼 在世界的中心呼喚愛 2012-10-15  

ls几位都说的很好。
我赞成你同事的写法，虽然多开销的一个list，但是action可读性变强了。何况你list=null的情况，返回一个null的list并不好，这样造成action都要去判断。
初期写代码尽量可读性好些，这样别人容易看懂，也容易给你一些意见。效率问题可以在后期慢慢调。

46 楼 michael8335 2012-10-15  

finallygo 写道

第一点我支持博主的,因为本来定义的接口就是可能多种实现的,虽然有的人说什么团队规范,但是一个团队规范不能保证所有人都认真的执行,其次还增加了一种潜规则在里面,所以,我采用不信任编程,另外第二个,我觉得还是尽量不要返回null

谢谢了，其实我写这个只是表达一下我的想法，同事想看看大家的看法，不过根据大家的评论来看，有两点是明显的，1、上层要做非空判断；2、下层不应该返回null。感谢大家的评论，从中学习不少。

45 楼 finallygo 2012-10-15  

第一点我支持博主的,因为本来定义的接口就是可能多种实现的,虽然有的人说什么团队规范,但是一个团队规范不能保证所有人都认真的执行,其次还增加了一种潜规则在里面,所以,我采用不信任编程,另外第二个,我觉得还是尽量不要返回null

44 楼 tobylxy 2012-10-15  

收获不少，谢lz和各位讨论！

43 楼 lycccxzt 2012-10-15  

虽不能提什么意见，但在各位的讨论中获得不少收获。

42 楼 formice 2012-10-15  

支持返回非null的集合，这里的问题关注点不在浪费那点内存上，而应该是程序的健壮性，不要太纠结理论，实际的应用应该灵活，不要把自己局限在条条框框上

41 楼 skzr.org 2012-10-15  

这个要顶-|-==============>

kidneyball 写道

谈谈返回null值的问题。在2009年，“null”的发明人就发表了一次演讲，说引入null这个概念是他犯下的一个巨大错误，几十年来造成了软件行业超过十亿美元的损失。（http://www.infoq.com/presentations/Null-References-The-Billion-Dollar-Mistake-Tony-Hoare）。因此我个人的倾向是，除非不得已，不要返回null。

以博主的场景为例，如果我在团队里规定，返回容器类型的方法不能返回null。那么对使用者来说，这个方法总共就三种合法情况：返回非空容器，返回空容器，抛出异常。如果发现返回值为null，那就说明是这个方法本身出错了，使用者可以直接反馈给编写者排错。

反之，如果团队里规定，“返回容器类型的方法里可能返回null”，那么对使用者来说，这个方法有以下几种可能情况：
1. 返回非空容器
2. 返回空容器
3. 抛出异常
4. 返回null，其意义相当于空容器
5. 返回null，其意义相当于抛异常
6. 返回null，有特殊的业务含义
7. 编写者没处理好，不小心返回了null（毕竟空指针是最常见的运行期错误，没有之一）。

使用者拿到一个这样的方法，就必须要在4、5、6、7之间做出选择。如果编写者没有写Javadoc，必须要看具体实现才能确认4和5，要加上一定推理才能确认6。无论选4,5,6，都无法排除可能是7。

40 楼 kidneyball 2012-10-15  

谈谈返回null值的问题。在2009年，“null”的发明人就发表了一次演讲，说引入null这个概念是他犯下的一个巨大错误，几十年来造成了软件行业超过十亿美元的损失。（http://www.infoq.com/presentations/Null-References-The-Billion-Dollar-Mistake-Tony-Hoare）。因此我个人的倾向是，除非不得已，不要返回null。

以博主的场景为例，如果我在团队里规定，返回容器类型的方法不能返回null。那么对使用者来说，这个方法总共就三种合法情况：返回非空容器，返回空容器，抛出异常。如果发现返回值为null，那就说明是这个方法本身出错了，使用者可以直接反馈给编写者排错。

反之，如果团队里规定，“返回容器类型的方法里可能返回null”，那么对使用者来说，这个方法有以下几种可能情况：
1. 返回非空容器
2. 返回空容器
3. 抛出异常
4. 返回null，其意义相当于空容器
5. 返回null，其意义相当于抛异常
6. 返回null，有特殊的业务含义
7. 编写者没处理好，不小心返回了null（毕竟空指针是最常见的运行期错误，没有之一）。

使用者拿到一个这样的方法，就必须要在4、5、6、7之间做出选择。如果编写者没有写Javadoc，必须要看具体实现才能确认4和5，要加上一定推理才能确认6。无论选4,5,6，都无法排除可能是7。

39 楼 lg_asus 2012-10-15  

原来我也是赞同楼主的做法，看样子我要看看effective java了。 thx