C++
Techniques de refactoring

Introduction

Refactoring à pied

Voici un programme qui pourrait bénéficier de la refactorisation. C'est un programme simple utilisant C ++ 11 qui est conçu pour calculer et imprimer tous les nombres premiers de 1 à 100 et est basé sur un programme qui a été publié sur CodeReview pour révision.

#include <iostream>
#include <vector>
#include <cmath>

int main()
{
    int l = 100;
    bool isprime;
    std::vector<int> primes;
    primes.push_back(2);
    for (int no = 3; no < l; no += 2) {
        isprime = true;
        for (int primecount=0; primes[primecount] <= std::sqrt(no); ++primecount) {
            if (no % primes[primecount] == 0) {
                isprime = false;
                break;
            } else if (primes[primecount] * primes[primecount] > no) {
                std::cout << no << "\n";
                break;
            }
        }
        if (isprime) {
            std::cout << no << " ";
            primes.push_back(no);
        }
    }
    std::cout << "\n";
}

La sortie de ce programme ressemble à ceci:

3 5 7 11 13 17 19 23 29 31 37 41 43 47 53 59 61 67 71 73 79 83 89 97

La première chose que nous remarquons est que le programme ne parvient pas à imprimer 2 qui est un nombre premier. Nous pourrions simplement ajouter une ligne de code pour simplement imprimer cette constante sans modifier le reste du programme, mais il serait peut-être préférable de refactoriser le programme pour le diviser en deux parties - une qui crée la liste de nombres premiers et une autre les imprime. . Voici comment cela pourrait ressembler:

#include <iostream>
#include <vector>
#include <cmath>

std::vector<int> prime_list(int limit)
{
    bool isprime;
    std::vector<int> primes;
    primes.push_back(2);
    for (int no = 3; no < limit; no += 2) {
        isprime = true;
        for (int primecount=0; primes[primecount] <= std::sqrt(no); ++primecount) {
            if (no % primes[primecount] == 0) {
                isprime = false;
                break;
            } else if (primes[primecount] * primes[primecount] > no) {
                break;
            }
        }
        if (isprime) {
            primes.push_back(no);
        }
    }
    return primes;
}

int main() 
{
    std::vector<int> primes = prime_list(100);
    for (std::size_t i = 0; i < primes.size(); ++i) {
        std::cout << primes[i] << ' ';
    }
    std::cout << '\n';
}

En essayant cette version, nous voyons que cela fonctionne effectivement correctement maintenant:

2 3 5 7 11 13 17 19 23 29 31 37 41 43 47 53 59 61 67 71 73 79 83 89 97

L'étape suivante consiste à noter que la deuxième clause if n'est pas vraiment nécessaire. La logique dans la boucle recherche les facteurs premiers de chaque nombre donné jusqu'à la racine carrée de ce nombre. Cela fonctionne parce que s'il y a des facteurs premiers d'un nombre, au moins l'un d'entre eux doit être inférieur ou égal à la racine carrée de ce nombre. Reprenant juste cette fonction (le reste du programme reste le même), nous obtenons ce résultat:

std::vector<int> prime_list(int limit)
{
    bool isprime;
    std::vector<int> primes;
    primes.push_back(2);
    for (int no = 3; no < limit; no += 2) {
        isprime = true;
        for (int primecount=0; primes[primecount] <= std::sqrt(no); ++primecount) {
            if (no % primes[primecount] == 0) {
                isprime = false;
                break;
            }
        }
        if (isprime) {
            primes.push_back(no);
        }
    }
    return primes;
}

Nous pouvons aller plus loin, changer les noms de variables pour les rendre un peu plus descriptifs. Par exemple, primecount n'est pas vraiment un décompte de nombres premiers. Au lieu de cela, il s'agit d'une variable d'index dans le vecteur des nombres premiers connus. De plus, alors que no est parfois utilisé comme abréviation pour "numéro", en écriture mathématique, il est plus courant d'utiliser n . Nous pouvons également apporter certaines modifications en éliminant la break et en déclarant les variables plus proches de leur utilisation.

std::vector<int> prime_list(int limit)
{
    std::vector<int> primes{2};
    for (int n = 3; n < limit; n += 2) {
        bool isprime = true;
        for (int i=0; isprime && primes[i] <= std::sqrt(n); ++i) {
            isprime &= (n % primes[i] != 0);
        }
        if (isprime) {
            primes.push_back(n);
        }
    }
    return primes;
}

Nous pouvons aussi refactoriser main pour utiliser un "range-for" pour le rendre un peu plus net:

int main() 
{
    std::vector<int> primes = prime_list(100);
    for (auto p : primes) {
        std::cout << p << ' ';
    }
    std::cout << '\n';
}

Ceci est juste un moyen de refactoring pourrait être fait. D'autres pourraient faire des choix différents. Cependant, le but du refactoring reste le même, à savoir améliorer la lisibilité et éventuellement les performances du code sans nécessairement ajouter de fonctionnalités.

Aller au nettoyage

Dans les bases de code C ++ qui étaient auparavant C, on peut trouver le modèle goto cleanup . Comme la commande goto rend le workflow d'une fonction plus difficile à comprendre, cela est souvent évité. Souvent, il peut être remplacé par des instructions de retour, des boucles, des fonctions. Cependant, avec le goto cleanup il faut se débarrasser de la logique de nettoyage.

short calculate(VectorStr **data) {
    short result = FALSE;
    VectorStr *vec = NULL;
    if (!data)
       goto cleanup;  //< Could become return false

    // ... Calculation which 'new's VectorStr

    result = TRUE;
cleanup:
    delete [] vec;
    return result;
}

En C ++, on pourrait utiliser RAII pour résoudre ce problème:

struct VectorRAII final {
    VectorStr *data{nullptr};
    VectorRAII() = default;
    ~VectorRAII() {
        delete [] data;
    }
    VectorRAII(const VectorRAII &) = delete;
};

short calculate(VectorStr **data) {
    VectorRAII vec{};
    if (!data)
       return FALSE;  //< Could become return false

    // ... Calculation which 'new's VectorStr and stores it in vec.data

    return TRUE;
}

À partir de là, on pourrait continuer à refactoriser le code actuel. Par exemple en remplaçant le VectorRAII par std::unique_ptr ou std::vector .