向左右向右走 —— 小时了了的技术博客

关注C++开发技术、架构设计、软件项目管理、软件产品管理等

posts - 19,comments - 55,trackbacks - 0

  事情缘起于代码走读会议的一次讨论。基本的需求很简单,就是实现类似下面这样一个函数:

HRESULT GetText(/* [out] */BSTR* pText);

  

  很自然地,首先写出来的实现是这样的:

HRESULT GetText(/* [out] */BSTR* pText)
{
	ASSERT(pText != NULL);
	*pText = ::SysAllocString(L"This is the text.");

	return S_OK;
}

  它的调用方式如下(这里为了简单起见我们忽略掉了对返回值的判断):

	BSTR bstText = NULL;
	GetText(&bstText);

  随即有人指出,可能出现另外的调用形式,如果不加以处理会造成内存泄漏:

	BSTR bstText = ::SysAllocString(L"This is another text.");
	// do something

	GetText(&bstText);

  于是函数实现被改成:

HRESULT GetText(/* [out] */BSTR* pText)
{
	ASSERT(pText != NULL);
	if (*pText != NULL)
	{
		::SysFreeString(*pText);
		*pText = NULL;
	}

	*pText = ::SysAllocString(L"This is the text.");
	return S_OK;
}

  看起来似乎完美了。但是,我们考虑下面一种情况:

	BSTR bstText;
	GetText(&bstText);

  注意上面的代码,它不太合乎规范,但在逻辑上却是合理的,声明一个变量并马上取它的指针调用函数赋值。问题来了,由于bstText会被初始化为一个随机值,所以直接导致if (*pText != NULL) 这行代码中判断失效,然后做了一次错误的内存释放。更严重的问题是这里代码可能正常执行却另外一个完全不相关的地方引发崩溃。程序员最怕的就是这种BUG,你可能花上一两天的时间调试追踪最后却只能靠逐行读代码的方式发现原来是某个角落的一行代码中变量未初始化或者错误的释放了一次内存。

  至于解决的方案,很多人首先想到的是对写出上面这种垃圾代码的程序员大声咆哮“声明变量一定要记住马上初始化”并把这一条款加入代码规范。可惜同样的错误依然会重复出现,菜鸟程序员可能会不记得规范,老鸟程序员同样可能疏忽或者仅仅是因为想偷一下懒,说到底,规范这种东西只是一种软约束,没有物理上的强制手段总是靠不住的。

  进一步分析可以看出上面的错误根源并不是因为变量未初始化,而是因为心理预期。很少有人会想到GetText这样一个函数中除了变量赋值之外还做了其他什么操作,即使想到了也要查看代码才能确认,如果每个函数都这样会让人发疯。你可以对别人咆哮可以写一堆的规范但改变不了别人的思维习惯。

  从代码健壮性的角度看,GetText函数的第二种实现比第一种脆弱的多,虽然它的内部逻辑更严密一些。改A处却因为B处的代码在C处引发崩溃,多么恐怖的图景啊,这也是很多规模大时间久的代码难于维护的原因。

  我们最终更改后GetText函数实现如下:

HRESULT GetText(/* [out] */BSTR* pText)
{
	ASSERT(pText != NULL);
	ASSERT(*pText == NULL);

	*pText = ::SysAllocString(L"This is the text.");
	return S_OK;
}

  这个方案的好处是,如果调用者传入的参数可能有问题(未初始化或者未释放现有内容)会马上给出警告,调用者必须立即修改自己的调用代码而不是等到程序崩溃时再去检查。唯一的缺点是调用起来有点麻烦,像下面这样:

	BSTR bstText = ::SysAllocString(L"This is another text.");
	// do something

	::SysFreeString(bstText);
	bstText = NULL;
	GetText(&bstText);

  

  最后总结一下本文的结论:

  1. 设计函数的内部逻辑时要考虑外部调用者的心理预期,但不要对调用形式做前提假设,任何可能出现的调用形式都可能出现;
  2. 如果传入参数可能有问题立即给出警告,鲁莽的容错处理可能比不做处理更加危险;
  3. 代码的健壮性不是孤立的也不是静态的,一段代码不仅内部逻辑要严密,而且要在外部代码发生变化的情况下保证内部逻辑的有效性;
  4. 修改一处代码却在另一处代码中引发错误,这是很多系统难以维护的原因,也是最令程序员抓狂的事,以上的三点都是为了极力避免出现这个问题。
本文地址:http://www.cnblogs.com/xrunning/archive/2011/10/30/2229166.html
posted on 2011-10-30 11:55 小时了了 阅读(...) 评论(...) 编辑 收藏