1

書き直そうとしているコードがいくつかあります。このコードは、さまざまな「ワークフロー」を必要とする多くの異なる呼び出し元が使用できるという点で、「汎用」になるように設計されています。次のようになります。

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 ファイルです。

このコードを維持/変更するのは大変な苦痛です。これを行うには、よりクリーンでシンプルで、頭脳爆発の少ない方法があるに違いないことを私は知っています!

4

3 に答える 3

3

XMLファイルの構造によって異なります。A / B / C / ...に個別にアクセスできれば、c ++/boostコードは次のようになります。

クラスFunctionalityAのすべてのA関連のもの、クラスFunctionalityBのB関連のものをリファクタリングします... FunctionalityProviderクラスは、システムの機能を構成するクラスです。TheOneAndOnlyMethodは、プロバイダーにすべての機能を要求し、それらを繰り返します。

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)));

}

A / B / Cに個別にアクセスできなかった場合は、パーサーにXMLファイルの内容に基づいて機能のリストを再表示させます。

于 2009-06-07T08:37:40.877 に答える
2

まず、各ifステートメントを、その機能を反映した名前のメソッドに変換します。次に、XMLファイルの内容に基づいてグローバルを設定する代わりに、ファイルを解析し、変数を介して通信するのではなく、適切なメソッドを順番に呼び出します。

于 2009-06-07T06:50:10.503 に答える
0

メソッドの名前は、問題/解決策への洞察を与えます：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);
    }
}
于 2009-06-07T06:51:19.547 に答える