2010-03-29 76 views
7

我有以下兩種方法(你可以看到)類似於其大部分語句除了一個(詳見下文)重構下面的兩個C++方法搬出來重複碼

unsigned int CSWX::getLineParameters(const SURFACE & surface, vector<double> & params) 
{ 
    VARIANT varParams; 

    surface->getPlaneParams(varParams); // this is the line of code that is different 

    SafeDoubleArray sdParams(varParams); 

    for(int i = 0 ; i < sdParams.getSize() ; ++i) 
    { 
     params.push_back(sdParams[i]); 
    } 

    if(params.size() > 0) return 0; 
    return 1; 
} 

unsigned int CSWX::getPlaneParameters(const CURVE & curve, vector<double> & params) 
{ 
    VARIANT varParams; 

    curve->get_LineParams(varParams); // this is the line of code that is different 

    SafeDoubleArray sdParams(varParams); 

    for(int i = 0 ; i < sdParams.getSize() ; ++i) 
    { 
     params.push_back(sdParams[i]); 
    } 

    if(params.size() > 0) return 0; 
    return 1; 
} 

有沒有什麼技術可以用來將兩種方法的代碼行移出到一個單獨的方法中,這可以從兩種方式中調用 - 或者 - 可能將兩種方法結合到一種方法中?

以下是限制:

  1. 的類曲面和曲線從第三方庫,因此不可修改。 (如果有幫助,他們都是從IDispatch接口派生)
  2. 還有更相似的類(例如FACE),可以融入這個「模板」(不是C++模板,行代碼只是流)

我知道下面可能(可能?)來實現的解決方案,但真的很希望有一個更好的解決方案:

  1. 我可以一個第三個參數添加到2種方法 - 例如:一個枚舉 - 標識第一個參數(例如enum :: input_type_surface,enum :: input_type_curve)
  2. 我可以傳入一個IDispatch並嘗試dynamic_cast <>並測試哪個轉換是NON_NULL並執行if-else來調用右邊方法(如getPlaneParams()與get_LineParams())

以下不是一個限制,但肯定是因爲我的隊友阻力的要求:

  1. 沒有實現從表面繼承一個新的類/ CURVE等(他們更願意使用上述的枚舉解決方案來解決它)
+1

你不清除'params'矢量。你打算用許多對象的參數來填充它嗎?也許有更好的方法來重構你的代碼,這取決於你之前調用geXXXXParameters方法的方法。 – 2010-03-29 17:41:33

+0

爲什麼在'bool'就夠了時返回'unsigned int'? – 2010-03-29 18:42:48

+0

「SafeDoubleArray」的類型是什麼?我懷疑這可能會重構更多,但我們首先需要。我第二@馬修的「bool」動議。 – GManNickG 2010-03-29 18:45:01

回答

10

一對夫婦的想法浮現在腦海中,但這裏是我覺得這是最好的:

namespace detail 
{ 
    void getParameters(const SURFACE& surface, VARIANT& varParams) 
    { 
     surface->getPlaneParams(varParams); 
    } 

    void getParameters(const CURVE& curve, VARIANT& varParams) 
    { 
     curve->get_LineParams(varParams); 
    } 
} 

template <typename T> 
unsigned int getParameters(const T& curve, vector<double> & params) 
{ 
    VARIANT varParams; 
    detail::getParameters(curve, varParams); 

    SafeDoubleArray sdParams(varParams); 
    for(int i = 0 ; i < sdParams.getSize() ; ++i) 
    { 
     params.push_back(sdParams[i]); 
    } 

    return params.size() != 0; 
} 

你要做的就是獲得委託參數,一個重載的一些其他功能的任務。只需爲每種不同類型添加類似的功能即可。 (注意,我簡化了你的返回語句。)

+0

不錯。我喜歡這個。我傾向於此。傷心我自己沒有想到這件事。 – ossandcad 2010-03-29 17:39:55

+1

非常好,特別是如果與Carl Manaster提到的'提取方法'一起移出一些代碼(調用'detail :: getParameters'後)。當我的模板方法很輕時,我更喜歡它:使得更輕的依賴關係圖:) – 2010-03-29 18:42:06

2

爲什麼不只是傳遞VARIANT varParams作爲參數傳遞給函數,而不是一個CURVESURFACE

unsigned int CSWX::getParameters(VARIANT varParams, vector<double> & params) 
{ 
    SafeDoubleArray sdParams(varParams); 

    for(int i = 0 ; i < sdParams.getSize() ; ++i) 
    { 
     params.push_back(sdParams[i]); 
    } 

    if(params.size() > 0) return 0; 
    return 1; 
} 

unsigned int CSWX::getPlaneParameters(const CURVE & curve, vector<double> & params) 
{ 
    VARIANT varParams;  

    curve->get_LineParams(varParams); // this is the line of code that is different 

    return getParameters(varParams, params); 
} 

您也可以考慮(如果可能的話),使這些方法的模板和接收output_iterator作爲參數,而不是一個vector。這樣你的代碼就不依賴於所用集合的類型。

+0

這會有幫助嗎? – Dave 2010-03-29 17:16:10

+0

對不起。我不太明白你的建議。由'表面'和'曲線'調用的方法是不同的 - 這是兩種方法之間不同的代碼行。而varParams是一個不需要的局部變量。也許我需要重申我的問題,如果沒有正確詢問?你能否提供一些示例代碼來幫助我理解? – ossandcad 2010-03-29 17:17:14

+0

爲了清晰起見,添加了代碼。這個解決方案會使它更短,特別是如果更多的方法受到影響。 – 2010-03-29 17:19:24

5

Extract Method。您標記爲不同的行之後的所有內容都是相同的 - 因此將其解壓爲從您的兩個原始方法中調用的方法。

+0

正確 - 儘可能多的通用代碼到單獨的方法。這工作。但是這是否意味着,如果我在「不同的代碼行」之前有更多的通用代碼,那麼「提取」它將會很困難? – ossandcad 2010-03-29 17:20:27

+0

基本上,任何重複的代碼塊都可以被提取到它自己的方法中,而不管有多少行會導致它,或者遵循它。例如,如果提取的方法需要太多參數,它可能會變得混亂,並且替代重構有時可能更合適。但提取方法是一個很好的開始。 – 2010-03-29 17:28:48

+0

您必須執行兩次提取方法(或更多,取決於有多少個地方有不同的代碼)。是的,這一點可能會變得困難。 – 2010-03-29 17:33:45

0

不是傳入一個SURFACE或CURVE,而是傳入一個對基類的引用,還有一個方法函數指針。然後調用surface-> getLine_parameters或curve-> getPlaneParamters將被替換爲(shape - > * getParamters)(varParams);

typedef void Baseclass::*ParamGetter(VARIANT varParams); 

unsigned int CSWX::getLineParameters(const Baseclass & geometry, ParamGetter getParams 
     vector<double> & params) 
{ 
    VARIANT varParams; 

    (geometry->*getParams)(varParams); // this is the line of code that is different 

    SafeDoubleArray sdParams(varParams); 

    for(int i = 0 ; i < sdParams.getSize() ; ++i) 
    { 
     params.push_back(sdParams[i]); 
    } 

    if(params.size() > 0) return 0; 
    return 1; 
} 
-2

宏!通常不是最好的解決方案(這是一個宏),並且可能不是這一個中最好的解決方案,但它可以完成這項工作。

macro_GetDakine_Params(func) 
    VARIANT varParams; \ 
    curve->##func(varParams); // this is the line of code that is different \ 
    SafeDoubleArray sdParams(varParams); \ 
    for(int i = 0 ; i < sdParams.getSize() ; ++i) \ 
    { \ 
     params.push_back(sdParams[i]); \ 
    } \ 
    if(params.size() > 0) return 0; \ 
    return 1; \ 

unsigned int CSWX::getPlaneParameters(const CURVE & curve, vector<double> & params) 
{ 
    macro_GetDakine_Params(getPlaneParams) 
} 
unsigned int CSWX::getLineParameters(const CURVE & curve, vector<double> & params) 
{ 
    macro_GetDakine_Params(getLineParams) 
} 
0

O!請不要使用竊聽聲明:

return params.size() != 0; 

它返回(-1)(中提出的所有位),因爲params是空的,沒有(1)。

但你真的可以整合這樣return語句:

return (params.size() != 0) ? 0 : 1; 

看來正確的類SURFACECURVE從抽象類FIGURESHAPE或部分多了一個導出。 所以你可以從模板中逃脫,並使用虛擬覆蓋:

void getParameters(const FIGURE& surface, VARIANT& varParams)