博客园  :: 首页  :: 新随笔  :: 联系 :: 订阅 订阅  :: 管理

一段代码重构引起的争议(一)

Posted on 2007-07-20 19:48  我是程序员  阅读(3157)  评论(18编辑  收藏  举报
    今天跟头就一段代码的问题展开了讨论,这段代码的主要作用是维护浏览器的浏览历史(这个浏览器是我们公司自己开发的Framework的一部分,主要的作用是在WinForm上实现类似Web浏览器的效果,我觉得很无聊的功能),如果不是很清楚的话可以看看IE或者FireFox的浏览动作列表就好了 。

这个类的大体接口是这个样子的:
 1 public class History
 2 {
 3    //属性,所有的历史项目列表
 4    public HistoryItemCollection Items;
 5   
 6    //历史项目后退一步
 7    public void Back();
 8   
 9    //历史项目前进一步
10    public void Forward();
11 
12    //清除历史内容
13    public void Clear();
14 
15    //跳到指定的序号
16    public void Goto( int index );
17 }

这个类的职责主要有以下两个:
  •     记录浏览历史(我觉得不太确切,但是文档中是这么说的,姑且就这么认为吧)。
  •     保持浏览历史页面的实例,当后退或者前进历史项目的时候,Explorer类通过从History类中获得页面的实例在容器中重新显示这个页面。
    手头没有Visio,所以就用文字来描述时序图了,以下描述了后退过程的调用时序图:
  1. 调用Explorer类的GetHistory方法获得当前Explorer的History
  2. 调用History的Back方法
  3. History获得当前位置的前一个位置的HistoryItem
  4. History调用Explorer类的方法显示HistoryItem.Instance(Instance为前一个位置页面的实例),在这个过程中History类触发页面的一些相关事件,例如BeforeNavigate、AfterNavigate等事件,并且调用当前显示页面的BoforeClose、AfterClose等事件
  5. History类调整历史项目位置的索引
  6. History类调用主界面的相关方法更新主界面上按钮状态和下列菜单中的内容

    具体设计的好坏我们就不在这里讨论了。本来这个类也工作了好几年了,本身也一直没有发生什么问题,所以也没有谁来关注这个类,虽然看起来有些...
    这个类大概写了有将近五年了,在此期间Framework发布了N多个版本,但是这个类一直没有什么变动。
    但是,随着时间的车轮隆隆的滚过,历史翻开了新的一页...
    突然有一天,有用户反应用我们的客户端浏览一定数目的页面后(40个左右,随机出现)真个客户端程序会突然毫无预兆的崩溃掉,我很不幸被指派来调查这个Bug。经过了大概两三天的调查,终于找到了问题出现的原因:User Object和GDI Object泄露!由于我们的程序美工上要求很高,用了一大堆的第三方控件(控件的名字我就不泄露了,免得引起不必要的法律纠纷),这些第三方控件除了DevExpress外,其它的都不是省油的灯,基本上一个控件用一次(就是创建一个实例),就能泄露一到两个UserObject和GDI Object。本来泄露GDI Object还好一点,毕竟每进程的配额数为9999个,就算配额用完了,大多数情况下也就是显示不正常,不会引起程序崩溃。但是User Object就不同了,一般的一个进程的User Object占用到6、7千个这个进程就铁定毫无预兆的崩溃了。偏偏我们页面上用的控件还超多,比较强悍一点的页面创建一次就会泄露两三百个User Object,真是很XXOO。
    控件肯定是没有办法改的,换控件也是不可能的,最终确定了修改方法:休克疗法。如果User Object一到达警戒线,就强制清历史项目(按照浏览的先后顺序,从旧到新的删除保存的实例,并且强制GC回收)直到User Object的数目回落到警戒线之下。
    经过这么一改使浏览的次数也就从四五十次到到了三四百次吧(记不太清了),虽然这个改法很是不匝地,但是效果还是刚刚的。
    于是,时间的车轮又隆隆的滚过,历史又翻开了新的一页。
    直到一天测试人员报了一个Bug,梦破碎了。
    俺改了这个类之后,历史列表的顺序反了,也就是说最新浏览的页面被扔到了历史列表的最后面。
    于是我又不得不去翻开History类的代码,最终找到了出问题的原因:
    在History类中,用两个栈(Stack类)来保存前进列表和后退列表,由于需要释放保存在这两个栈中的HistoryItem实例,所以需要将栈中的内容全部导出到数组中,处理完后将残余的实例再压回栈。虽然我在代码中写了大段的注释提醒不要把顺序给搞反了,结果我还是搞反了,我直到现在都觉得当时是不是想哪位美女了,心不在焉,把栈的顺序给搞错了。
    由于代码已经Release到各个产品组了,要改代码是很慎重的问题,于是就需要和头讨论如何操刀来改,这就是这个讨论的由来。
    回头一看竟然发现没有写一点点重构的内容,不过今晚有事要去接老婆了,也只能到这里了,欲知后事请听下回分解。
    未完待续
    PS:今天竟然不小心成为了骗子,郁闷