リファクタリング繰り返しIf文ブロック

Question

私は書き直そうとしているコードをいくつか持っています。このコードは、さまざまな「ワークフロー」を必要とする多くの異なる発信者が使用できるという点で、「汎用」に設計されています。これは次のようなものです：

string globalA; 
int globalB; 
bool globalC; 
// Lots more globals in various shapes and forms 

void TheOneAndOnlyMethod(XmlBasedConfig config) { 

    // Set all of the globals based on XML configuration 
    // ... 

    if (globalA.Length > 0) 
     // Do something relating to functionality 'a' 
    } 

    if (globalB > 0) { 
     // Do something relating to functionality 'b' 
    } 

    if (globalC) { 
     // You get the idea 
    } 
} 

発信者の中には、globalAとglobalBが設定されているため、関連するifブロックにあるものはすべて実行されます。他の発信者には、何が必要なのかを行うための無数の設定があります。呼び出し元は基本的に単なる設定のXMLファイルです。

このコードの維持/変更は、大きなの痛みです。私はクリーナー、シンプル、少ない脳が爆発する必要があります知っているこれを行う方法！

Answer 1

これはXMLファイル構造に依存します。もし私がA/B/C /にアクセスできるなら... ...別に私のC++/boostコードはこのように見えます。

FunctionalityAのクラスA関連のすべてをリファクタリングする 、FunctionalityBのクラスB関連... FunctionalityProviderクラスは、システムの機能を設定するクラスです。 OneAndOnlyMethodは、プロバイダにすべての機能を問い合わせ、それらの機能を反復処理します。私は別々に/ B/Cにアクセスすることができなかった場合は

class XmlFunctionality 
{ 
public: 
    virtual ~XmlFunctionality(){ 
    } 
    virtual void loadFromConfig(XmlBasedConfig) = 0; 
    virtual bool isEnabled() const = 0; 
    virtual void execute() = 0; 

protected: 
    XmlFunctionality(){ 
    }; 
} 

class FunctionalityA : public XmlFunctionality 
{ 
public: 
    void loadFromConfig(XmlBasedConfig){ 
     // load A information from xml 
    } 
    bool isEnabled() const{ 
     return globalA.length()>0; // is A a global !?  
    } 
    void execute(){ 
     // do you're a related stuff 
    } 
} 

class FunctionalityB : public XmlFunctionality 
{ 
public: 
    void loadFromConfig(XmlBasedConfig){ 
     // load B information from xml 
    } 
    bool isEnabled() const{ 
     // when is b enabled ...  
    } 
    void execute(){ 
     // do you're b related stuff 
    } 
} 

// Map your question to the functions - 
class FunctionalityProvider 
{ 
    Functionality functionalityList; 
public: 
    typedef std::vector<XmlFunctionality*> Functionality; 
    FunctionalityProvider() : functionalityList() {   
     functionalityList.push_back(new FunctionalityA); 
     functionalityList.push_back(new FunctionalityB); 
     functionalityList.push_back(...); 
    } 

    ~FunctionalityProvider {   
     for_each(functionality.begin(), functionality.end(), delete_ptr()); 
    } 

    Functionality functionality() const { 
     return functionalityList; 
    } 
} 
void TheOneAndOnlyMethod(XmlBasedConfig config, const FunctionalityProvider& provider) { 
    const FunctionalityProvider::Functionality functionality = provider.functionality(); 
    for_each( 
     functionality.begin(), 
     functionality.end(), 
     bind(&XmlFunctionality::loadFromConfig, _1, config)); // or some other way of parsing the file 
    for_each( 
     functionality.begin(), 
     functionality.end(), 
     if_then(bind(&XmlFunctionality::isEnabled, _1), bind(&XmlFunctionality::execute, _1))); 

} 

は、私は、XMLファイルの内容に基づいて機能のリストをreturingパーサをしましょう。

Answer 2

各ifステートメントを、それが何を反映している名前のメソッドに変えることから始めますか。次に、XMLファイルの内容に基づいてグローバルを設定するのではなく、変数を介して通信するのではなく、ファイルを解析して適切なメソッドを呼び出します。

Answer 3

あなたのメソッドの名前は、問題/解決策のいくつかの洞察を提供します：TheOneAndOnlyMethod。非常に特定の操作を処理し、再利用可能な小さなメソッドにコードを分割する必要があるように思えます。

string globalA; 
int globalB; 
bool globalC; 
// Lots more globals in various shapes and forms 

void TheOneAndOnlyMethod(XmlBasedConfig config) { 
    // Set all of the globals based on XML configuration 
    loadXmlAsGlobals(config); 

    if (globalA.Length > 0) 
     methodOne(globalA); 
     methodTwo(globalA); 
    } 

    if (globalB > 0) { 
     methodTwo(globalB); 
     methodThree(globalB); 
    } 

    if (globalC) { 
     methodOne(globalC); 
     methodFour(globalC); 
    } 
}