`
zhang_xzhi_xjtu
  • 浏览: 538836 次
  • 性别: Icon_minigender_1
  • 来自: 杭州
社区版块
存档分类
最新评论

实践中的重构19_脱裤子放屁

阅读更多
每当看到代码中有一个明显的冗余的时候,我就有一个感慨,这家伙时间真多啊,放个屁还要脱裤子。
看例子。
		if (addressCode != null
				&& (StringUtil.equals(addressCode, HK)
						|| StringUtil.equals(addressCode, MACAO)
						|| StringUtil.equals(addressCode, TAIWAN) || StringUtil
						.equals(addressCode, OTHER))) {
			// do something.
		} else {
			// do something.
		}

其中StringUtil是一个null safe的方法,HK,MACAO,TAIWAN都是常量定义,后面明明有判断相等的逻辑,偏偏要在前面做一个null检查,真是多此一举。
		if (StringUtil.equals(addressCode, HK)
				|| StringUtil.equals(addressCode, MACAO)
				|| StringUtil.equals(addressCode, TAIWAN)
				|| StringUtil.equals(addressCode, OTHER)) {
			// do something.
		} else {
			// do something.
		}

直接去掉不是挺好的吗?
同时,从这里使用常量的方式可以推测,有可能其他地方也有基于比较的用法。代码文件往下一拉,果不其然。
		if (StringUtil.equals(addressCode, HK)) {
			return "中国香港";
		}
		if (StringUtil.equals(addressCode, MACAO)) {
			return "中国澳门";
		}
		if (StringUtil.equals(addressCode, TAIWAN)) {
			return "中国台湾";
		}
		if (StringUtil.equals(addressCode, OTHER)) {
			return "其它国家/地区";
		}

两处放在一起考虑,使用map来存储addressCode和地址描述字符串就是一个水到渠成的事情了。
第一处的代码可以改为:
		if (addressMap.containsKey(addressCode)) {
			// do something.
		} else {
			// do something.
		}

第二处的代码可以改为:
		if (addressMap.containsKey(addressCode)) {
			return addressMap.get(addressCode);
		}

当然,这里addressCode对应的描述信息应该从配置文件里面读取比较好。但是由于不是本文关注的重点,不赘述。
分享到:
评论
25 楼 lichuan 2011-03-19  


代码风格问题
24 楼 ugibb510 2011-03-07  
抛出异常的爱 写道
zhang_xzhi_xjtu 写道
抛出异常的爱 写道
去他的 扯淡的优化原则......

以好读为准...


一堆if else或者 equals 好读,还是一个contains好读。

enum更爽快一些....
hashmap还要put一堆东东好恶心



enum很爽,但1.4的JDK编译不通过!
23 楼 抛出异常的爱 2011-03-07  
mathgl 写道
抛出异常的爱 写道
zhang_xzhi_xjtu 写道
抛出异常的爱 写道
去他的 扯淡的优化原则......

以好读为准...


一堆if else或者 equals 好读,还是一个contains好读。

enum更爽快一些....
hashmap还要put一堆东东好恶心


switch 更好些,不过现在还没搞定string...

如果语法里默认含有break switch 也可以用用.....

有了break后太灵活了
22 楼 aninfeel 2011-03-07  
这种太平码在我们的项目太tmd常见了,如if(str!=null&&StringUtils.isNotEmpty(str)),还有一些验证之后再验证的,写代码的人明显是怕出事,宁可多判也不少判,只要程序不出错就行。这种人在程序界往往混得很好。而有代码洁癖的却常常出问题(一般都是调用别人的方法造成的),反而在boss面前显得不可靠。
21 楼 mathgl 2011-03-07  
抛出异常的爱 写道
zhang_xzhi_xjtu 写道
抛出异常的爱 写道
去他的 扯淡的优化原则......

以好读为准...


一堆if else或者 equals 好读,还是一个contains好读。

enum更爽快一些....
hashmap还要put一堆东东好恶心


switch 更好些,不过现在还没搞定string...
20 楼 volking 2011-03-05  
一样脱裤子放.
19 楼 抛出异常的爱 2011-03-04  
zhang_xzhi_xjtu 写道
抛出异常的爱 写道
去他的 扯淡的优化原则......

以好读为准...


一堆if else或者 equals 好读,还是一个contains好读。

enum更爽快一些....
hashmap还要put一堆东东好恶心
18 楼 zhang_xzhi_xjtu 2011-03-04  
抛出异常的爱 写道
去他的 扯淡的优化原则......

以好读为准...


一堆if else或者 equals 好读,还是一个contains好读。
17 楼 抛出异常的爱 2011-03-04  
去他的 扯淡的优化原则......

以好读为准...
16 楼 zhang_xzhi_xjtu 2011-03-04  
Ben.Sin 写道
buzhucele 写道
ak121077313 写道
if (StringUtil.equals(addressCode, HK)) { 
    return "中国香港"; 
}  else{
if (StringUtil.equals(addressCode, MACAO)) { 
    return "中国澳门"; 
}  else{
if (StringUtil.equals(addressCode, TAIWAN)) { 
    return "中国台湾"; 
}  else{
if (StringUtil.equals(addressCode, OTHER)) { 
    return "其它国家/地区"; 
} }}
}
.........

这种写法要是效率上应该比用Map高,不过返回一个字符变量在代码的可读性和易易维护性要好一些。


如果第一个就命中是if..else效率比较高,但是如果后面的命中,Map明显比if..else效率高

package ben.test.performance.ifelse;

import java.util.HashMap;
import java.util.Map;

public class MapIfEnum {
	public static final String HK = "HK";
	public static final String MACAO = "MC";
	public static final String TW = "TW";
	public static final String OTHER = "OTHER";
	
	public static void main(String[] args){
//		String code = HK;
//		String code = MACAO;
//		String code = TW;
		String code = OTHER;
		MapIfEnum test = new MapIfEnum();
		long start = 0L + System.currentTimeMillis();
		for (int i = 0; i < 1000000; i ++){
			test.useIfElse(code);
//			test.useEnum(code);
//			test.useMap(code);
		}
		long end = 0L + System.currentTimeMillis();
		System.out.println("Used time = " + (end - start));
	}
	
	public String useIfElse(String code){
		if (HK.equals(code)){
			return "中国香港";
		} else if (MACAO.equals(code)){
			return "中国澳门";
		} else if (TW.equals(code)){
			return "中国台湾";
		} else {
			return "其他地区";
		}
	}
	
	public String useEnum(String code){
		return Area.valueOf(code).getDesc();
	}
	
	public String useMap(String code){
		return areaMap.get(code);
	}
	
	enum Area{
		HK("中国香港"),
		MC("中国澳门"),
		TW("中国台湾"),
		OTHER("其他地区");
		
		private String desc;
		
		private Area(String desc){
			this.desc = desc;
		}
		
		public String getDesc(){
			return desc;
		}
	}
	
	public static Map<String, String> areaMap = new HashMap<String, String>();
	static {
		areaMap.put("HK", "中国香港");
		areaMap.put("MC", "中国澳门");
		areaMap.put("TW", "中国台湾");
		areaMap.put("OTHER", "其他地区");
	}
}



特地试了一下,排除异常的情况,每1,000,000此循环
if..else 根据命中几率递增,第一个最快15ms,第二个命中基本上与Map持平23ms,到第三个比Enum稍快31ms
Map      基本上保持在24ms左右
Enum     基本上在36ms左右


还没有来得及出性能测试呢,被仁兄抢先了。
15 楼 zhang_xzhi_xjtu 2011-03-04  
Willam2004 写道
一般java源文件中是不推荐直接写中文,可以通过资源文件来进行管理,是不是更好。

同意,但是不是本文的关注点。
14 楼 zhang_xzhi_xjtu 2011-03-04  
ak121077313 写道
if (StringUtil.equals(addressCode, HK)) { 
    return "中国香港"; 
}  else{
if (StringUtil.equals(addressCode, MACAO)) { 
    return "中国澳门"; 
}  else{
if (StringUtil.equals(addressCode, TAIWAN)) { 
    return "中国台湾"; 
}  else{
if (StringUtil.equals(addressCode, OTHER)) { 
    return "其它国家/地区"; 
} }}
}
.........

代码格式化一下看看。
13 楼 zhang_xzhi_xjtu 2011-03-04  
neo_q 写道
这段代码别的全貌不好说,但是就null判断还是有意义的,

1.可以以短路的形式减少对API的调用;

2.对于使用的API方法,你做出了null safe的假设(尽管你可能是看了源码或者读了它的doc),而事实上,一个API是不应该有假设的,也就是说你作为调用者不应当假设你所调用的接口满足某些条件或者前提,只有这样子接口之间的依赖才会小,达成低耦合的效果;对于假设而言又或者在1.0版本是null safe,2.0不是了,那么你是否要改代码?所以在调用外部API时,最好不要做出假设,这也是OO设计的原则。


1 同意,由于场景的实际情况,调用的时候最多的情况不为null,考虑情况不应该这么写。
2 不同意。api已经注明了是null safe。另,一个方法,如果你不相信它的某些条件或者前提,这个方法你就不应该调用。著名的例子就是2分搜索。2.0如果改动的话,因为涉及到接口定义的改动,应该新建一个方法。
12 楼 zhang_xzhi_xjtu 2011-03-04  
Crusader 写道
zhang_xzhi_xjtu 写道
每当看到代码中有一个明显的冗余的时候,我就有一个感慨,这家伙时间真多啊,放个屁还要脱裤子。
看例子。
		if (addressCode != null
				&& (StringUtil.equals(addressCode, HK)
						|| StringUtil.equals(addressCode, MACAO)
						|| StringUtil.equals(addressCode, TAIWAN) || StringUtil
						.equals(addressCode, OTHER))) {
			// do something.
		} else {
			// do something.
		}

其中StringUtil是一个null safe的方法,HK,MACAO,TAIWAN都是常量定义,后面明明有判断相等的逻辑,偏偏要在前面做一个null检查,真是多此一举。
		if (StringUtil.equals(addressCode, HK)
				|| StringUtil.equals(addressCode, MACAO)
				|| StringUtil.equals(addressCode, TAIWAN)
				|| StringUtil.equals(addressCode, OTHER)) {
			// do something.
		} else {
			// do something.
		}

直接去掉不是挺好的吗?
同时,从这里使用常量的方式可以推测,有可能其他地方也有基于比较的用法。文件往下一拉,果不其然。
		if (StringUtil.equals(addressCode, HK)) {
			return "中国香港";
		}
		if (StringUtil.equals(addressCode, MACAO)) {
			return "中国澳门";
		}
		if (StringUtil.equals(addressCode, TAIWAN)) {
			return "中国台湾";
		}
		if (StringUtil.equals(addressCode, OTHER)) {
			return "其它国家/地区";
		}

两处放在一起考虑,使用map来存储addressCode和地址描述字符串就是一个水到渠成的事情了。
		if (addressMap.containsKey(addressCode)) {
			// do something.
		} else {
			// do something.
		}

		if (addressMap.containsKey(addressCode)) {
			return addressMap.get(addressCode);
		}



		if (addressCode != null
				&& (StringUtil.equals(addressCode, HK)
						|| StringUtil.equals(addressCode, MACAO)
						|| StringUtil.equals(addressCode, TAIWAN) || StringUtil
						.equals(addressCode, OTHER))) {
			// do something.
		} else {
			// do something.
		}


addressCode如果为null的话,直接短路,至少可以减少一次方法调用啊
有些人处处都很注意性能,习惯问题,也没到脱裤子放屁那么严重


鉴于公司原因代码不能全部开放,考虑性能的话就不会这么写了,因为不为null的情况是最多的。
11 楼 Ben.Sin 2011-03-04  
buzhucele 写道
ak121077313 写道
if (StringUtil.equals(addressCode, HK)) { 
    return "中国香港"; 
}  else{
if (StringUtil.equals(addressCode, MACAO)) { 
    return "中国澳门"; 
}  else{
if (StringUtil.equals(addressCode, TAIWAN)) { 
    return "中国台湾"; 
}  else{
if (StringUtil.equals(addressCode, OTHER)) { 
    return "其它国家/地区"; 
} }}
}
.........

这种写法要是效率上应该比用Map高,不过返回一个字符变量在代码的可读性和易易维护性要好一些。


如果第一个就命中是if..else效率比较高,但是如果后面的命中,Map明显比if..else效率高

package ben.test.performance.ifelse;

import java.util.HashMap;
import java.util.Map;

public class MapIfEnum {
	public static final String HK = "HK";
	public static final String MACAO = "MC";
	public static final String TW = "TW";
	public static final String OTHER = "OTHER";
	
	public static void main(String[] args){
//		String code = HK;
//		String code = MACAO;
//		String code = TW;
		String code = OTHER;
		MapIfEnum test = new MapIfEnum();
		long start = 0L + System.currentTimeMillis();
		for (int i = 0; i < 1000000; i ++){
			test.useIfElse(code);
//			test.useEnum(code);
//			test.useMap(code);
		}
		long end = 0L + System.currentTimeMillis();
		System.out.println("Used time = " + (end - start));
	}
	
	public String useIfElse(String code){
		if (HK.equals(code)){
			return "中国香港";
		} else if (MACAO.equals(code)){
			return "中国澳门";
		} else if (TW.equals(code)){
			return "中国台湾";
		} else {
			return "其他地区";
		}
	}
	
	public String useEnum(String code){
		return Area.valueOf(code).getDesc();
	}
	
	public String useMap(String code){
		return areaMap.get(code);
	}
	
	enum Area{
		HK("中国香港"),
		MC("中国澳门"),
		TW("中国台湾"),
		OTHER("其他地区");
		
		private String desc;
		
		private Area(String desc){
			this.desc = desc;
		}
		
		public String getDesc(){
			return desc;
		}
	}
	
	public static Map<String, String> areaMap = new HashMap<String, String>();
	static {
		areaMap.put("HK", "中国香港");
		areaMap.put("MC", "中国澳门");
		areaMap.put("TW", "中国台湾");
		areaMap.put("OTHER", "其他地区");
	}
}



特地试了一下,排除异常的情况,每1,000,000此循环
if..else 根据命中几率递增,第一个最快15ms,第二个命中基本上与Map持平23ms,到第三个比Enum稍快31ms
Map      基本上保持在24ms左右
Enum     基本上在36ms左右
10 楼 Willam2004 2011-03-04  
一般java源文件中是不推荐直接写中文,可以通过资源文件来进行管理,是不是更好。
9 楼 buzhucele 2011-03-04  
ak121077313 写道
if (StringUtil.equals(addressCode, HK)) { 
    return "中国香港"; 
}  else{
if (StringUtil.equals(addressCode, MACAO)) { 
    return "中国澳门"; 
}  else{
if (StringUtil.equals(addressCode, TAIWAN)) { 
    return "中国台湾"; 
}  else{
if (StringUtil.equals(addressCode, OTHER)) { 
    return "其它国家/地区"; 
} }}
}
.........

这种写法要是效率上应该比用Map高,不过返回一个字符变量在代码的可读性和易易维护性要好一些。
8 楼 ak121077313 2011-03-04  
if (StringUtil.equals(addressCode, HK)) { 
    return "中国香港"; 
}  else{
if (StringUtil.equals(addressCode, MACAO)) { 
    return "中国澳门"; 
}  else{
if (StringUtil.equals(addressCode, TAIWAN)) { 
    return "中国台湾"; 
}  else{
if (StringUtil.equals(addressCode, OTHER)) { 
    return "其它国家/地区"; 
} }}
}
.........
7 楼 neo_q 2011-03-04  
这段代码别的全貌不好说,但是就null判断还是有意义的,1.可以以短路的形式减少对API的调用;2.对于使用的API方法,你做出了null safe的假设(尽管你可能是看了源码或者读了它的doc),而事实上,一个API是不应该有假设的,也就是说你作为调用者不应当假设你所调用的接口满足某些条件或者前提,只有这样子接口之间的依赖才会小,达成低耦合的效果;对于假设而言又或者在1.0版本是null safe,2.0不是了,那么你是否要改代码?所以在调用外部API时,最好不要做出假设,这也是OO设计的原则。
6 楼 Crusader 2011-03-04  
zhang_xzhi_xjtu 写道
每当看到代码中有一个明显的冗余的时候,我就有一个感慨,这家伙时间真多啊,放个屁还要脱裤子。
看例子。
		if (addressCode != null
				&& (StringUtil.equals(addressCode, HK)
						|| StringUtil.equals(addressCode, MACAO)
						|| StringUtil.equals(addressCode, TAIWAN) || StringUtil
						.equals(addressCode, OTHER))) {
			// do something.
		} else {
			// do something.
		}

其中StringUtil是一个null safe的方法,HK,MACAO,TAIWAN都是常量定义,后面明明有判断相等的逻辑,偏偏要在前面做一个null检查,真是多此一举。
		if (StringUtil.equals(addressCode, HK)
				|| StringUtil.equals(addressCode, MACAO)
				|| StringUtil.equals(addressCode, TAIWAN)
				|| StringUtil.equals(addressCode, OTHER)) {
			// do something.
		} else {
			// do something.
		}

直接去掉不是挺好的吗?
同时,从这里使用常量的方式可以推测,有可能其他地方也有基于比较的用法。文件往下一拉,果不其然。
		if (StringUtil.equals(addressCode, HK)) {
			return "中国香港";
		}
		if (StringUtil.equals(addressCode, MACAO)) {
			return "中国澳门";
		}
		if (StringUtil.equals(addressCode, TAIWAN)) {
			return "中国台湾";
		}
		if (StringUtil.equals(addressCode, OTHER)) {
			return "其它国家/地区";
		}

两处放在一起考虑,使用map来存储addressCode和地址描述字符串就是一个水到渠成的事情了。
		if (addressMap.containsKey(addressCode)) {
			// do something.
		} else {
			// do something.
		}

		if (addressMap.containsKey(addressCode)) {
			return addressMap.get(addressCode);
		}



		if (addressCode != null
				&& (StringUtil.equals(addressCode, HK)
						|| StringUtil.equals(addressCode, MACAO)
						|| StringUtil.equals(addressCode, TAIWAN) || StringUtil
						.equals(addressCode, OTHER))) {
			// do something.
		} else {
			// do something.
		}


addressCode如果为null的话,直接短路,至少可以减少一次方法调用啊
有些人处处都很注意性能,习惯问题,也没到脱裤子放屁那么严重

相关推荐

Global site tag (gtag.js) - Google Analytics