Refactoring répété Si blocs d'instructions

Question

J'ai du code que j'essaie de réécrire. Le code est conçu pour être "générique", en ce sens qu'il peut être utilisé par de nombreux appelants différents qui nécessitent des "flux de travail" différents. Il va quelque chose comme ceci:

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

Certains appelants ont mis Globala et globalB, fait donc tout ce qui est lié dans le cas des blocs. Les autres appelants auront une myriade d'autres paramètres pour faire ce qu'ils doivent faire. Les appelants sont essentiellement un fichier XML avec des paramètres.

Le maintien/la modification de ce code est une douleur big. Je sais qu'il doit y avoir un moyen plus propre, plus simple et moins explosif de faire cela!

Answer 1

Cela dépend de votre structure de fichier XML. Si je pouvais accéder à A/B/C/... séparément, mon code C++/boost ressemblerait à ceci.

Refactoriser toutes les choses liées à une classe dans FunctionalityA , B choses liées à la classe FunctionalityB, ... classe FunctionalityProvider est la classe vous permet de configurer les fonctionnalités de votre système. TheOneAndOnlyMethod demande au fournisseur toutes les fonctionnalités et effectue une itération sur celles-ci.

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

} 

Si je ne pouvais pas accéder à A/B/C séparement je laisse un analyseur returing une liste de fonctionnalités basées sur le contenu du fichier XML.

Answer 2

Je commencerais par transformer chaque instruction if en une méthode avec un nom reflétant ce qu'il fait. Ensuite, au lieu de définir des globales en fonction du contenu du fichier XML, analysez le fichier et appelez les méthodes appropriées dans l'ordre plutôt que de communiquer via des variables.

Answer 3

Le nom de votre méthode donne un aperçu du problème/de la solution: TheOneAndOnlyMethod. Il semble que vous ayez besoin de casser votre code en plusieurs petites méthodes qui traitent chacune des opérations très spécifiques et qui sont également réutilisables.

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