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 回答