实践中的重构06_方法调用顺序和性能

大部分的书在讲解涉及到性能优化的时候都告诉我们不要过早优化。简而言之，过早优化是万恶之源。
过早的担心性能问题，不经过性能测试，同时花费大量的时间和精力去在一些主观判断的性能瓶颈点做了一些性能优化的工作，实际上很有可能对性能提高没有什么帮助，而且导致代码可读性直线下降。这样的做法当然是不明智的。
但是，打着过早优化是万恶之源的幌子，从而在写代码的时候压根不考虑性能问题，以为计算机就是万能机器，没有性能问题，一样是不可取的。
在写code的时候，性能还是应该有所考虑的。而且很多时候，对性能问题的考虑并不会花费很多时间，同时也不会把代码搞的一团糟。

	public boolean f_0(String userId) {

		// isLoginAgainInMonth is local call.
		boolean isLoginAgainInMonth = isLoginAgainInMonth(userId);
		// isGardUser is ws call.
		boolean isGardUser = isGardUser(userId);

		return isLoginAgainInMonth || isGardUser;
	}

上面的code判断了一个用户是本月的非第一次登录，或者是一个受保护的用户，是的话返回true，做一些处理。
大眼一看，这段代码当然是没有什么问题的。但是加上性能的考量，问题就出来了。
isLoginAgainInMonth是一个local call，isGardUser是一个ws call，当isLoginAgainInMonth为true的时候是可以直接返回的，多调用一次isGardUser就多一次ws开销，这个开销包括网络开销，内存开销，甚至db开销。如果该代码被大量执行次数放大的话，所消耗的资源还是很可观的。
这里调整代码来强调方法的调用顺序对性能的影响。重构后为：

	/**
	 * Note: 为了性能考量，先调用本地方法，后调用ws服务，当可以判断出结果的时候，尽早返回。
	 * */
	public boolean f_1(String userId) {
		// isLoginAgainInMonth is local call.
		if (isLoginAgainInMonth(userId)) {
			return true;
		}

		// isGardUser is ws call.
		return isGardUser(userId);
	}

还有一种利用短路特性的写法。

	/**
	 * 不推荐的写法。
	 * */
	public boolean f_2(String userId) {
		return isLoginAgainInMonth(userId) || isGardUser(userId);
	}

这种写法利用短路的特性达到了同样的功能，并且看上去代码更简洁。
但是，应该承认的是，并不是所有程序员对短路的特性都掌握了，同时，即使掌握短路的程序员看到这段代码的时候也未必能够意识到这里的短路和性能相关。该写法没有达到强调方法调用顺序和性能的关系的作用，因此不推荐这种写法。

评论

33 楼 ldbjakyo 2010-11-18  

tuti 写道

zhang_xzhi_xjtu 写道

tuti 写道

zhang_xzhi_xjtu 写道

		//isLastLoing is local call.
		boolean isLoginAgainInMonth=isLoginAgainInMonth(userId);
		
		if(isLoginAgainInMonth){
			return true;
		}
		
		//isGardUser is ws call.
		boolean isGardUser=isGardUser(userId);
		
		return isGardUser;

这样如何？

	
		return isLoginAgainInMonth(userId)||isGardUser(userId);

这个当然也行，最早我也是这个想法，但是考虑到未必所有的人都知道短路，才用长一点的方式写，这样可以突出早返回这个特性，后来的程序员也就不会忽略这个东西了。

这样如何？

		
		if(is_login_again_in_month(userId)){
			return true;
		}
		return is_gard_user_via_call_ws(userId);

这样很不错的， 我觉得这个有些像 fowler提倡的  replace temp with query

32 楼 抛出异常的爱 2010-11-17  

iamlotus 写道

tuti 写道

月落码农 写道

代码的自我注释的确是很好，这也是大家推荐的做法。
   但是它也有不足，它不可能100%的说明自身，最多60，70%，在很多的模糊场景下，有的隐含条件我觉得必须写明注释，要不一两个月后，也许你自己忘了，更别说别人维护。

有个东西叫做自动测试

自动测试又不测性能的
这种地方既然为了优化，还是写个注释吧。如果不写，这段代码我是看不出楼主刻意强调的用意的

性能测试我也写过
@timeout

zhang_xzhi_xjtu 写道

lewisw 写道

首先我想说的是你这个不是重构， 起码不是代码重构， 而是逻辑的重构。


重构的前提是在不改变代码逻辑的前提下，改给代码换一种结构，一增加可读性、可扩展性。。。。。。


你当前的重构明显改变了逻辑，就是第一个方法调用结果如果为true， 就不调用第二个了，或许你后面的isGardUser方法的调用本身也是必须的， 比如会在远程的服务器上生成验证记录什么的。那你这样去掉反而不对。

你现在的重构实际上就是逻辑上重新整理。

不要局限于M的重构定义。

还是要在意的,
不过改变逻辑需要带另一个帽子而已

31 楼 zhang_xzhi_xjtu 2010-11-16  

tuti 写道

iamlotus 写道

tuti 写道

月落码农 写道

代码的自我注释的确是很好，这也是大家推荐的做法。
   但是它也有不足，它不可能100%的说明自身，最多60，70%，在很多的模糊场景下，有的隐含条件我觉得必须写明注释，要不一两个月后，也许你自己忘了，更别说别人维护。

有个东西叫做自动测试

自动测试又不测性能的
这种地方既然为了优化，还是写个注释吧。如果不写，这段代码我是看不出楼主刻意强调的用意的

写注释不是问题，
问题是注释要写成什么样。
怎么保证注释和代码能同步更新。

写注释是可以接受的。
但是短路不接受。

30 楼 zhang_xzhi_xjtu 2010-11-16  

lewisw 写道

首先我想说的是你这个不是重构， 起码不是代码重构， 而是逻辑的重构。


重构的前提是在不改变代码逻辑的前提下，改给代码换一种结构，一增加可读性、可扩展性。。。。。。


你当前的重构明显改变了逻辑，就是第一个方法调用结果如果为true， 就不调用第二个了，或许你后面的isGardUser方法的调用本身也是必须的， 比如会在远程的服务器上生成验证记录什么的。那你这样去掉反而不对。

你现在的重构实际上就是逻辑上重新整理。

不要局限于M的重构定义。

29 楼 tuti 2010-11-16  

iamlotus 写道

tuti 写道

月落码农 写道

代码的自我注释的确是很好，这也是大家推荐的做法。
   但是它也有不足，它不可能100%的说明自身，最多60，70%，在很多的模糊场景下，有的隐含条件我觉得必须写明注释，要不一两个月后，也许你自己忘了，更别说别人维护。

有个东西叫做自动测试

自动测试又不测性能的
这种地方既然为了优化，还是写个注释吧。如果不写，这段代码我是看不出楼主刻意强调的用意的

写注释不是问题，
问题是注释要写成什么样。
怎么保证注释和代码能同步更新。

28 楼 iamlotus 2010-11-16  

tuti 写道

月落码农 写道

代码的自我注释的确是很好，这也是大家推荐的做法。
   但是它也有不足，它不可能100%的说明自身，最多60，70%，在很多的模糊场景下，有的隐含条件我觉得必须写明注释，要不一两个月后，也许你自己忘了，更别说别人维护。

有个东西叫做自动测试

自动测试又不测性能的
这种地方既然为了优化，还是写个注释吧。如果不写，这段代码我是看不出楼主刻意强调的用意的

27 楼 tuti 2010-11-16  

月落码农 写道

代码的自我注释的确是很好，这也是大家推荐的做法。
   但是它也有不足，它不可能100%的说明自身，最多60，70%，在很多的模糊场景下，有的隐含条件我觉得必须写明注释，要不一两个月后，也许你自己忘了，更别说别人维护。

有个东西叫做自动测试

26 楼 月落码农 2010-11-16  

gdpglc 写道

chiaun 写道

我认为LZ应该是想让程式码自我注释，如果程式码可以自我注释我认为比明确写上注释要好

不必如果，楼主的代码很明确，你认为达没达到你说的 自我注释的要求？

代码的自我注释的确是很好，这也是大家推荐的做法。
   但是它也有不足，它不可能100%的说明自身，最多60，70%，在很多的模糊场景下，有的隐含条件我觉得必须写明注释，要不一两个月后，也许你自己忘了，更别说别人维护。

25 楼 cleanerje 2010-11-16  

建议弄个简单的权限框架来搞定这个事情，这样维护也很easy。

24 楼 gdpglc 2010-11-16  

trydofor 写道

感觉问题除性能外，还在于：
* WS 或 Heavy方法，在API或项目内没有重视（管理问题）
* 原代码质量不好，接手较痛苦（如，result 魔术） （培训问题）

什么是result魔术？

23 楼 月落码农 2010-11-16  

xuzhfa123 写道

已经想过头了，利用短路方式已经够了。

认可

22 楼 trydofor 2010-11-16  

感觉问题除性能外，还在于：
* WS 或 Heavy方法，在API或项目内没有重视（管理问题）
* 原代码质量不好，接手较痛苦（如，result 魔术） （培训问题）

21 楼 lewisw 2010-11-16  

首先我想说的是你这个不是重构， 起码不是代码重构， 而是逻辑的重构。


重构的前提是在不改变代码逻辑的前提下，改给代码换一种结构，一增加可读性、可扩展性。。。。。。


你当前的重构明显改变了逻辑，就是第一个方法调用结果如果为true， 就不调用第二个了，或许你后面的isGardUser方法的调用本身也是必须的， 比如会在远程的服务器上生成验证记录什么的。那你这样去掉反而不对。

你现在的重构实际上就是逻辑上重新整理。

20 楼 tuti 2010-11-16  

有空都去读一下
《重构》，
第9章 简化条件表达式 （Simplifying Conditional Expressions）

19 楼 dabian_guo 2010-11-16  

首先，所有的书告诉你的应该是：不要过早优化。

其次，你那个只是设计失误而已，算不上性能优化。

18 楼 gdpglc 2010-11-16  

chiaun 写道

我认为LZ应该是想让程式码自我注释，如果程式码可以自我注释我认为比明确写上注释要好

不必如果，楼主的代码很明确，你认为达没达到你说的 自我注释的要求？

17 楼 chiaun 2010-11-16  

gdpglc 写道

zhang_xzhi_xjtu 写道

dwbin 写道

其实觉得代码还是以易读为主，向这样的代码宁肯多写两行，短路的写法是最高效的，但是如果出了问题需要进行单步调试的话还是像楼主那样写起来比较方便。
但是这样写也有个坏处，就是第一个条件被做为了第二个判断的启动条件，不太利于以后的理解，建议还是短路，不过开始的时候增加一个中间变量。
boolean isLoginAgainInMonth=isLoginAgainInMonth(userId);     
return isLoginAgainInMonth||isGardUser(userId);
这样单步调试的时候比较容易控制

这里的纠结在于大家都明白短路，可以不能指望所有人都明白。

当后来的人维护的时候，很有可能加一个条件，取一个值，放到短路条件的第一个。
如果加的条件是开销很大的调用的话，就悲剧了。

分开写，是为了提醒维护的人，这里的调用顺序是重要的。

看到a||b||c，一般人的反应是，三个条件有一个为true，则返回，而不会想调用顺序的问题。

如果这个真的很重要，为什么不写点注释...

我认为LZ应该是想让程式码自我注释，如果程式码可以自我注释我认为比明确写上注释要好

16 楼 gdpglc 2010-11-16  

zhang_xzhi_xjtu 写道

dwbin 写道

其实觉得代码还是以易读为主，向这样的代码宁肯多写两行，短路的写法是最高效的，但是如果出了问题需要进行单步调试的话还是像楼主那样写起来比较方便。
但是这样写也有个坏处，就是第一个条件被做为了第二个判断的启动条件，不太利于以后的理解，建议还是短路，不过开始的时候增加一个中间变量。
boolean isLoginAgainInMonth=isLoginAgainInMonth(userId);     
return isLoginAgainInMonth||isGardUser(userId);
这样单步调试的时候比较容易控制

这里的纠结在于大家都明白短路，可以不能指望所有人都明白。

当后来的人维护的时候，很有可能加一个条件，取一个值，放到短路条件的第一个。
如果加的条件是开销很大的调用的话，就悲剧了。

分开写，是为了提醒维护的人，这里的调用顺序是重要的。

看到a||b||c，一般人的反应是，三个条件有一个为true，则返回，而不会想调用顺序的问题。

如果这个真的很重要，为什么不写点注释...

15 楼 zhang_xzhi_xjtu 2010-11-16  

dwbin 写道

其实觉得代码还是以易读为主，向这样的代码宁肯多写两行，短路的写法是最高效的，但是如果出了问题需要进行单步调试的话还是像楼主那样写起来比较方便。
但是这样写也有个坏处，就是第一个条件被做为了第二个判断的启动条件，不太利于以后的理解，建议还是短路，不过开始的时候增加一个中间变量。
boolean isLoginAgainInMonth=isLoginAgainInMonth(userId);     
return isLoginAgainInMonth||isGardUser(userId);
这样单步调试的时候比较容易控制

这里的纠结在于大家都明白短路，可以不能指望所有人都明白。

当后来的人维护的时候，很有可能加一个条件，取一个值，放到短路条件的第一个。
如果加的条件是开销很大的调用的话，就悲剧了。

分开写，是为了提醒维护的人，这里的调用顺序是重要的。

看到a||b||c，一般人的反应是，三个条件有一个为true，则返回，而不会想调用顺序的问题。

14 楼 zhang_xzhi_xjtu 2010-11-16  

明天的昨天 写道

这个也考虑过，只是为了方便调试，一般把方法的返回值都独立出来。

我也想知道到底方便在哪里?
楼主的上一个帖子，不是也直接这样了吗？


实践中的重构05_简洁的代码

if (a|| b|| c) {
   return true;
}
return false;

可以改为

return a|| b|| c;

一个是调用方法，一个是变量，不一样。