设计不佳的代价——顺序耦合性Bug案例一则
温 昱 (wenyu@china.com)
2003年11月
本文已发表于《CSDN开发高手》2003年12期
Bug发现得越晚，修正的成本就越高。本文的例子就是一个很好的证明。
本文首先对顺序耦合性做一个简单介绍；然后讲述笔者在实际工作中遇到的一个“顺序耦合性Bug”，这种Bug通过代码走查很难发现，常需求诸于跟踪调试才行；最后通过重构改进设计，从根本上解除顺序耦合性隐患。
一、顺序耦合性简介
耦合性（Coupling）是两个元素之间的一种关系，为了保持正确性，需要两个元素同时进行变化。耦合性可以分为静态耦合性、动态耦合性和差异耦合性3大类。
顺序耦合性是静态耦合性的一种，为了保持正确性，两个元素必须以特定的顺序出现。
例如，下列两条语句的顺序，反映了变量“先定义后使用”的要求，不能颠倒。
int i;
i = 6 ;
再例如，下列两条语句的顺序，也不能颠倒。
FILE * fp = fopen( “a.txt” );
fcolse( fp );
二、遭遇顺序耦合性Bug
1、项目简介
这是一个多媒体编辑软件，要求编辑的结果可以保存为“项目”，以便以后打开或进一步编辑。经简化的软件架构如下图所示。问题领域层（PD）对多媒体编辑领域进行了抽象，数据管理层（DM）负责数据的保存和读取，用户界面层（UI）负责为用户提供图形化用户界面。并且，DM层和UI层并不相互依赖，它们都仅依赖于PD层，这样的软件架构将PD层、DM层和UI层最大限度地独立开来，有利于应对需求的变更。很自然地，由3个相对独立的开发小组，分别负责上述3个子系统。
2、新的需求
原先版本的保存和打开功能，都是针对专有格式的二进制“项目”文件。在新的版本中，要求支持XML格式的“项目”文件。
由于软件架构设计的合理性，支持这项新需求的工作相当简单。首先，PD小组和DM小组一起，制定XML文件的格式，形成文档。然后，DM小组指派XML高手，写一个叫做CXmlSaver的类去实现CProjectSaver接口，该类按照XML文件格式文档，将Project Model保存到XML中去；同理，还要写一个叫做CxmlLoader的类去实现CProjectLoader接口；如下图所示。最后，UI小组做少量工作，增加相关功能的界面。
3、发现Bug
哈，我就是DM小组中被指派写CXmlLoader的家伙。但是很不幸，我的下列代码有Bug，不能将XML文件中的Video File的信息正确地Load到Project Model中的Video File节点。
void CXmlLoader::LoadVideoFile(CVideoFile * inVideo)
{
...
inVideo->SetStartTime( GetNodeAttr(Video, "startTime") );
inVideo->SetEndTime( GetNodeAttr(Video, "endTime") );
inVideo->SetDuration( GetNodeAttr(Video, "duration") );
...
} |
4、跟踪调试
这到底是怎样回事呢？我进行了一番代码走查，并没有发现什么不合理的地方。
于是，我在inVideo->SetEndTime( GetNodeAttr(Video, "endTime") );处设置断点，然后跟踪到void CVideoFile::SetEndTime(double inEndTime)里面去。我看到了下面的代码。
CVideoFile::CVideoFile()
{
...
mDuration = 0;
...
}
void CVideoFile::SetEndTime(double inEndTime)
{
if (inEndTime > mDuration)
mEndTime = mDuration;
else if (inEndTime > 0)
mEndTime = inEndTime;
else
mEndTime = 0;
} |
查看一下变量当前的值。呀，此处的mDuration还保持着构造函数中为它赋的初值0哟，而inEndTime的值是324.6。于是，if (inEndTime > mDuration)成立，所以mEndTime被设置为mDuration的值0。
再跟踪到inVideo->SetDuration( GetNodeAttr(Video, "duration") );里面看看，我看到了下面的代码。
void CVideoFile::SetDuration(double inDuration)
{
if (inDuration > 0)
mDuration = inDuration;
else
mDuration = 0;
} |
看一下变量当前的值。咦，这里inDuration的值是600.0，倒是比刚才要设置的EndTime的值324.6大，显然，刚才inVideo->SetEndTime( GetNodeAttr(Video, "endTime") );中，应该用这个Duration值来进行合法性检查的。喔，调用inVideo->SetDuration( GetNodeAttr(Video, "duration") );晚了，应该在inVideo->SetEndTime( GetNodeAttr(Video, "endTime") );之前就调用的。
原因找到了，只需调整一下语句的顺序，Bug就被修正了。代码如下所示。
void CXmlLoader::LoadVideoFile(CVideoFile * inVideo)
{
...
inVideo->SetDuration( GetNodeAttr(Video, "duration") );
inVideo->SetStartTime( GetNodeAttr(Video, "startTime") );
inVideo->SetEndTime( GetNodeAttr(Video, "endTime") );
...
} |
三、通过重构改进设计
从表面上看，Bug已经被修正。但是，顺序耦合性的隐患还在；一不留神，同样的Bug还会出现。下面，我们通过重构改进设计，从根本上解除顺序耦合性隐患。
1、分析原因
先来分析一下CVideoFile的设计思想。作为Project Model的一部分，PD小组实现了一个严格检查合法性的CVideoFile，任何时候都不允许一个Video File的结束时间比其总时间大。
再看问题出在哪里。从CVideoFile本身提供的接口，看不出顺序耦合性的存在——我无从知道必须先调用CVideoFile::SetDuration()，后调用CVideoFile::SetEndTime()。
2、解决策略
设计一个进行严格检查合法性的CVideoFile，是合理的，应当保留。
但是CVideoFile的接口设计的不合理。我们应当改进CVideoFile的接口设计，消除其中的顺序耦合性隐患，使用户在不知道先设置Duration后设置EndTime的情况下，也不会犯错误。
下面我们通过一步步地重构，来达到这个目的。
3、运用重构的“Extract Method”成例
重构的“Extract Method”成例建议，如果一个代码段拥有完整的意义，则应当将它们单独抽出，放入一个方法，并根据其意义为方法命名。
下列使用CVideoFile的代码段，正是这样的情况；而且，如果不加改进，很容易出现顺序耦合性Bug。
void CXmlLoader::LoadVideoFile(CVideoFile * inVideo)
{
...
inVideo->SetDuration( GetNodeAttr(Video, "duration") );
inVideo->SetStartTime( GetNodeAttr(Video, "startTime") );
inVideo->SetEndTime( GetNodeAttr(Video, "endTime") );
...
} |
但是，我们决定对CVideoFile本身进行“Extract Method”重构，而不是对CVideoFile的客户代码，因为这样所有使用CVideoFile的客户代码都可以从中受益。我们为CVideoFile增加了这样一个公共方法：
void CVideoFile::SetTribleTime(double inStartTime,
double inEndTime, double inDuration)
{
SetDuration(inDuration);
SetStartTime(inStartTime);
SetEndTime(inEndTime);
} |
4、运用重构的“Hide Method”成例
重构的“Hide Method”成例建议，如果一个方法并不被除所属类之外的其它类使用，那么该方法应当是private方法。
作为CVideoFile的设计者，我希望客户程序调用SetTribleTime()去设置相关属性，并不希望客户程序直接调用SetDuration()、SetStartTime()和SetEndTime()中的任何一个。所以我将这3个方法置为private的。
5、运用重构的“Introduce Parameter Object”成例
重构的“Introduce Parameter Object”成例建议，如果一个方法的多个参数可以很自然地划分在一起，就应当用一个对象来代替它们。
方法void CVideoFile::SetTribleTime(double inStartTime, double inEndTime, double inDuration)的3个参数显然满足这样的条件，所以，我们可以引入一个CTribleTime类来代替它们，代码如下。
class CTribleTime
{
double duration;
double startTime;
double endTime;
};
void CVideoFile::SetTribleTime(CTribleTime & inTribleTime)
{
SetDuration(inTribleTime.duration);
SetStartTime(inTribleTime.startTime);
SetEndTime(inTribleTime.endTime);
} |
6、其它改进
现在的设计并不完美，因为我们在进行多媒体编辑的时候，可能只需要设置Start Time，这时代码会象下面所示，很繁琐。
CTribleTime t;
t.startTime = 200;
t.endTime = video.GetEndTime();
t.duration = video.GetDuration();
video.SetTribleTime( t ); |
下面，我们为CVideoFile增加一个GetTribleTime()方法，通过它可以方便地取得CVideoFile的3个Time的当前值。最后，我们仅设置Start Time的代码看起来象这样。
CTribleTime t = video.GetTribleTime();
t.startTime = 200;
video.SetTribleTime( t ); |
另外，我们可以将CVideoFile的3个成员变量mStartTime、mEndTime和mDuration改用一个CTribleTime mTribleTime实现；删除GetDuration()、GetStartTime()和GetEndTime()这3个方法；等等。
参考文献：
《重构——改善既有代码的设计（影印版）》 Martin Fowler
《UML面向对象设计基础》 Meilir Page-Jones著 包晓露等译
