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