Рефакторинг повторяющихся блоков 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
    }
}

У некоторых вызывающих абонентов установлены глобальныеA и globalB, поэтому они выполняют все, что находится в связанных блоках if. Другие абоненты будут иметь множество других настроек, чтобы делать все, что им нужно. Абоненты - это просто XML-файл с настройками.

Поддержание / изменение этого кода - боль большая . Я знаю, что должен быть более чистый, простой, менее взрывающий мозг способ сделать это!

Answer 1

Это зависит от вашей файловой структуры XML. Если бы я мог получить доступ к A / B / C / ... по отдельности, мой код на c ++ / boost выглядел бы так.

Рефакторинг всех связанных с A вещей в классе FunctionalityA , B связанные вещи в классе FunctionalityB, ... Класс 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.

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