论坛首页 Java企业应用论坛

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

浏览 9003 次
精华帖 (0) :: 良好帖 (0) :: 新手帖 (4) :: 隐藏帖 (4)
作者 正文
   发表时间:2011-03-03   最后修改:2011-06-06
每当看到代码中有一个明显的冗余的时候,我就有一个感慨,这家伙时间真多啊,放个屁还要脱裤子。
看例子。
		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对应的描述信息应该从配置文件里面读取比较好。但是由于不是本文关注的重点,不赘述。
   发表时间:2011-03-03  
if (HK.equals(addressCode)  
        || MACAO.equals(addressCode)  
        || TAIWAN.equals(addressCode)  
        || OTHER.equals(addressCode)) {  
    // do something.  
} else {  
    // do something.  
}  


And then...

if (isAddressRegistered(addressCode)) {  
    // do something.  
} else {  
    // do something.  
}  

boolean isAddressRegistered(addressCode)
{
    return addressMap.find(addressCode);
}
0 请登录后投票
   发表时间:2011-03-03  
哈哈  你是wxz吧  看到头像想起你来了
0 请登录后投票
   发表时间:2011-03-03  
这个抽象出一个小方法,应该说是有一定道理的。
但是containsKey作为一个底层api,语义是比较饱满的,所以我不想抽一个小方法出来。

这个道理如同有几个地方调用数组的length一样。一般不会抽一个方法出来的。

当然,这个是个人风格问题。

0 请登录后投票
   发表时间:2011-03-03  
leversss 写道
哈哈  你是wxz吧  看到头像想起你来了

头像的功能还是很强大的啊。
0 请登录后投票
   发表时间:2011-03-04  
原作者的写法,效率要高一些。
1 请登录后投票
   发表时间: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的话,直接短路,至少可以减少一次方法调用啊
有些人处处都很注意性能,习惯问题,也没到脱裤子放屁那么严重
0 请登录后投票
   发表时间:2011-03-04  
这段代码别的全貌不好说,但是就null判断还是有意义的,1.可以以短路的形式减少对API的调用;2.对于使用的API方法,你做出了null safe的假设(尽管你可能是看了源码或者读了它的doc),而事实上,一个API是不应该有假设的,也就是说你作为调用者不应当假设你所调用的接口满足某些条件或者前提,只有这样子接口之间的依赖才会小,达成低耦合的效果;对于假设而言又或者在1.0版本是null safe,2.0不是了,那么你是否要改代码?所以在调用外部API时,最好不要做出假设,这也是OO设计的原则。
1 请登录后投票
   发表时间: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 "其它国家/地区"; 
} }}
}
.........
0 请登录后投票
   发表时间: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高,不过返回一个字符变量在代码的可读性和易易维护性要好一些。
0 请登录后投票
论坛首页 Java企业应用版

跳转论坛:
Global site tag (gtag.js) - Google Analytics