代碼的壞味道
3、Large Class(過大類)
如果想利用單一class做太多事情,其內往往就會出現太多instance變量。一旦如此,Duplicated Code也就接踵而至了。
解決辦法:
(1)Extract Class:將數個變量一起提煉至新class內,提煉時應該選擇彼此相關的變量。
當某個class做了應該由兩個class做的事的時候使用。
建立一個新的class,將相關的值域和函數從舊class搬移到新class.
步驟:
1)決定如何分解class所負責任.
2)建立一個新class,用以表現從舊class分離出來的責任.
3)建立從舊class訪問新class的連接關系.
4)對于你想搬移的每一個值域,用Move Field搬移之.
5)每次搬移后,編譯,測試.
6)使用Move Method將必要的函數搬移到新class,先搬移較低層次函數,即被調用次數多余調用其他函數次數的函數.再搬移較高層次的函數.
7)每次搬移之后,編譯,測試.
8)檢查,精簡每個class接口.
9)決定是否讓新class曝光.
為了加深對于重構的理解,我開始尋找能夠實踐這些方法的代碼.終于,我找到了大學時期寫的有關圖形處理的一個小程序,并打算對這個程序進行重構.從某些方面講,它還是比較合適的:
1、程序寫得很爛,但是還能工作;
2、包含了大部分書中提到的“壞味道”。
3、有關位圖等圖形文件的格式信息和相關的規約還有算法,我已經忘得差不多了,除了知道這些代碼是我自己寫得之外,我已經基本上不認識它們了。這樣我就可以實踐書中所說的“邊重構,邊理解”的過程了。
4、我寫這個程序的時候還比較勤快,寫了不少注釋。
首先,我找到最長的那個類,并繪制了類圖:

這里面成員變量夠多。其中有一些只不過是聲明了一下;還有一些初始化了,但一次也沒有使用過。
首先用這個類來實踐Extract Class的重構方法。
首先,我把其中有關直方圖的成員和方法獨立出來,形成直方圖類,以變成下面這種形式:

雖然很丑陋,但是還是先看一下在類CBasicBmpProcess中的InitHistoGram()方法:
void CBasicBmpProcess::InitHistoGram()//初始化直方圖
{
long i;
int j,BmpSize=bi.biWidth*bi.biHeight,ColorCount=bitCount<<8;
if(ColorCount<=256)
{
for(i=0;i<ColorCount;i++)
{
HistoGram[i]=0.0;
iHistoGram[i]=0;
}
for(i=0;i<BmpSize;i++)
{
iHistoGram[lpBuf[i]]++;
}
for(i=0;i<ColorCount;i++)
{
HistoGram[i]=((float)iHistoGram[i])/BmpSize;
}
}
else
{
for(i=0;i<ColorCount;i++)
{
HistoGramExt[i]=0.0;
iHistoGramExt[i]=0;
}
for(i=0;i<BmpSize-bitCount;i=i+bitCount)
{
for(j=0;j<bitCount;j++)
{
iHistoGramExt[j*ColorCount/bitCount+lpBuf[i+j]]++;
}
}
for(i=0;i<ColorCount;i++)
{
HistoGramExt[i]=10*((float)iHistoGramExt[i])/(BmpSize/bitCount);
}
}
}
現在要把它抽取到新類CHistoGram中去。像下面這樣:
void CHistoGram::InitHistoGram(int BmpSize,int ColorCount,BYTE *lpBuf,int bitCount)
{
long i=0,j=0;
if(ColorCount<=256)
{
for(i=0;i<ColorCount;i++)
{
HistoGram[i]=0.0;
iHistoGram[i]=0;
}
for(i=0;i<BmpSize;i++)
{
iHistoGram[lpBuf[i]]++;
}
for(i=0;i<ColorCount;i++)
{
HistoGram[i]=((float)iHistoGram[i])/BmpSize;
}
}
else
{
for(i=0;i<ColorCount;i++)
{
HistoGramExt[i]=0.0;
iHistoGramExt[i]=0;
}
for(i=0;i<BmpSize-bitCount;i=i+bitCount)
{
for(j=0;j<bitCount;j++)
{
iHistoGramExt[j*ColorCount/bitCount+lpBuf[i+j]]++;
}
}
for(i=0;i<ColorCount;i++)
{
HistoGramExt[i]=10*((float)iHistoGramExt[i])/(BmpSize/bitCount);
}
}
}
然后再將CBasicBmpProcess中的InitHistoGram方法變為下面這樣:
void CBasicBmpProcess::InitHistoGram()//初始化直方圖
{
int BmpSize=bi.biWidth*bi.biHeight,ColorCount=bitCount<<8;
this->_histoGram.InitHistoGram(BmpSize,ColorCount,lpBuf,bitCount);
}
然后繼續使用Extract Class對CBasicBmpProcess進行抽離。在此過程當中,由于CBasicBmpProcess已經失去了最初的功能,所以將它重命名為CBmpData。效果如下圖所示:

調用舉例:
void CBmpData::AveStrainWaves()//均值濾波
{
this->processor->AveStrainWaves();
BmpStatsChanged=TRUE;
}
(2)Extract SubClass
class 中的某些特性只被某些(而非全部)實體用到。
Extract Class和Extract Sub Class是重構的兩種選擇,兩者之間的抉擇實際上就是委托和繼承之間的抉擇。Extract Sub Class通常更容易進行,但它也有限制:一旦對象創建完成,你無法再改變與“型別相關的行為”。但如果使用Extract Class,你只需要插入另外一個不同組件,就可以改變對象行為。此外,subclass只能表現一組變化,如果你希望class以數種不同的方式變化,就必須使用委托。
作法:
為Source Class定義一個新的Sub Class;
為這個新的Sub Class定義構造函數:
簡單的做法是:讓Sub Class構造函數與Super Class有相同的參數,并通過Super調用Super Class構造函數.
如果你希望對用戶隱藏Sub Class的存在,可以使用Replace Constructor With Factory Method.
找出調用Super Class的所有地點,如果它們需要的是新建的Sub Class,讓它們改而調用新的構造函數;
如果Sub Class的構造函數和super class構造函數的參數不同,可以使用Rename Method修改其參數列。如果Sub Class構造函數不需要Super Class構造函數的某些參數,可以使用Rename Method將它們去除。
(Rename Method:這里我的理解是:需要創建具體子類實例的時候,用一個名稱清晰的函數去創建它,并隱藏因為構造函數不同而導致實例化時的差別。)
如果不在需要直接實體化,就將它聲明為抽象類。
逐步使用Push Down Method和Push Down Field將Source Class特性移到Sub Class中去。
和Extract Class不同的是,先處理函數再處理數據,通常會簡單一些。
當一個public函數被下移到Sub Class后,你可能需要重新定義該函數的調用端的局部變量或參數型別,讓它們調用SubClass中的新函數,如果忘記進行這一步驟,編譯器會提醒你。
找到所有這樣的值域:它們所傳達的信息如今可由繼承體系自身傳達,以Self Encapsulate Field避免直接使用這些值域,然后將它們的取值函數替換為多態常量函數,所有使用這些值域的地方都應該以Replace Conditional with Polymorphism重構。
(多態常量函數:會在不同的subclass版本中返回不同的固定值)
在我的圖像處理程序中,我發現圖像處理器類(CBmpProcessor)中存在一些可以分離的特性。比如均值濾波絕對不會和索貝爾邊緣檢測扯上什么關系,所以我打算對Processor類使用Extract Sub Class方法。讓它變為下面這種形式:

然后把Process變為一個純虛函數,這樣,CBmpProcessor就成為一個抽象類了,同時還需要對調用方--即類CBasicBmpData進行修改,另外由于這個修改改動了CBasicBmpProcess暴露的公共方法,所以要調用這些方法的窗口中的事件處理函數也需要進行一定修改。首先CBmpProcessor看上去會是這個樣子:

這樣在SetBmpProcessor的函數體中配置Processor:
void CBmpData::SetBmpProcessor(CBmpProcessor *processor)
{
this->processor=processor;
this->processor->SetBitmapInfoHeader(bi);
this->processor->SetRawData(lpBuf);
this->processor->PrepareHistoGram(this->_histoGram);
this->processor->PrepareProcessingData(this->lptmpBuf);
}
而調用Processor處理圖像就更簡單了:
void CBmpData::ProcessByProcessor(void)
{
this->processor->Process();
}
然后把窗口中的事件處理函數修改一下:
void CMainFrame::OnSobel()
{
// TODO: Add your command handler code here
if(WindowCounter>0)
{
CMDIChildWnd* pMDIActive = MDIGetActive();
CImageDoc* pDoc=(CImageDoc*)pMDIActive->GetActiveDocument();
pDoc->bmp.SetBmpProcessor(new CSobel());
pDoc->bmp.ProcessByProcessor();
pDoc->UpdateAllViews(NULL,0,NULL);
}
else
AfxMessageBox("請先打開一幅位圖!");
}
完整的CBmpProcessor類圖:

(3)Extract Interface
若干客戶使用class接口中的同一子集;或者兩個classes的接口有部分相同。
處理方法:將相同的子集提煉到一個獨立的接口中。
在許多面向對象語言中,這種“責任劃分能力”是通過多重繼承支持的,你可以針對一段行為建立一個class,再將它們組合于一份實現品中。java只提供單一繼承,但你可以運用interface來昭示并實現這種需求。
如果某個class在不同環境中扮演截然不同的角色,使用interface是一個好主意。
作法:
新建一個空接口;
在接口中聲明待提煉類的共同操作;
讓相關的class實現上述接口;
調整客戶的型別聲明,使得以運用該接口。
(4)Duplicate Observed Data:如果你的Large Class是一個GUI class,你可能需要把數據和行為移動到一個獨立的領域對象去。
將Domain Data拷貝到一個domain object中,建立一個Observer模式,用以對Domain object和GUI object內的數據進行同步控制。
一個分層良好的系統,應該將處理用戶界面和處理業務邏輯的代碼分開。之所以這樣做,原因有以下幾點:
(1)你可能需要使用數個不同的用戶界面來表現相同的業務邏輯:如果同時承擔兩種責任,用戶界面會變得過分復雜;
(2)與GUI隔離之后,domain object的維護和演化都會更容易:你甚至可以讓不同的開發者負責不同部分的開發。
作法:
修改presentation class,使其成為domain class的觀察者。
針對GUI class內的domain data,使用Self Encapsulate Field。
編譯,測試
在事件處理函數中加上對設值函數的調用,以“直接訪問方式”更新GUI組件。
編譯、測試。
在domain class中定義數據及其相關的訪問函數。
修改presentation class中的訪問函數,將它們的操作對象改為domain object。
修改observer的update,使其從相應的domain object中所需的數據拷貝給GUI組件。
編譯,測試。
看上去很像.net2005中的DataObject。在前臺綁定GridView之后,可以將GridView中的一些行為(添加、修改、刪除)與DataObject進行映射。比如:
<asp:ObjectDataSource ID="ObjectDataSource2" runat="server" SelectMethod="GetUsersInRole"
TypeName="System.Web.Security.Roles" OldValuesParameterFormatString="original_{0}" OnSelecting="ObjectDataSource2_Selecting">
<SelectParameters>
<asp:ControlParameter ControlID="GridView1" Name="roleName" PropertyName="SelectedValue"
Type="String" />
</SelectParameters>
</asp:ObjectDataSource>
代碼:
[DataObject]
public static class RoleDB
{
[DataObjectMethod(DataObjectMethodType.Select)]
public static RoleInfoCollection GetAllRoles()
{
RoleInfoCollection infos = new RoleInfoCollection();
string[] roleNames = Roles.GetAllRoles();
foreach (string roleName in roleNames)
{
RoleInfo info = new RoleInfo();
info.Name = roleName;
info.Symbol = roleName;
infos.Add(info);
}
return infos;
}


posted on 2007-07-01 00:10
littlegai 閱讀(424)
評論(0) 編輯 收藏 引用