C++
Tecniche di refactoring

introduzione

Ristrutturazione a piedi attraverso

Ecco un programma che potrebbe beneficiare del refactoring. È un programma semplice che utilizza C ++ 11 che ha lo scopo di calcolare e stampare tutti i numeri primi da 1 a 100 e si basa su un programma che è stato pubblicato su CodeReview per la revisione.

#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";
}

L'output di questo programma è simile a questo:

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 prima cosa che notiamo è che il programma non riesce a stampare 2 che è un numero primo. Potremmo semplicemente aggiungere una riga di codice per stampare semplicemente una costante senza modificare il resto del programma, ma potrebbe essere più ordinario rifattorizzare il programma in modo da dividerlo in due parti: una che crea l'elenco dei numeri primi e un'altra che le stampa . Ecco come potrebbe apparire:

#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';
}

Provando questa versione, vediamo che funziona davvero correttamente ora:

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

Il prossimo passo è notare che la seconda clausola if non è realmente necessaria. La logica nel ciclo cerca i fattori primi di ogni dato numero fino alla radice quadrata di quel numero. Questo funziona perché se ci sono dei fattori primi di un numero almeno uno di essi deve essere inferiore o uguale alla radice quadrata di quel numero. Rielaborando solo quella funzione (il resto del programma rimane lo stesso) otteniamo questo risultato:

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

Possiamo andare oltre, cambiando i nomi delle variabili per essere un po 'più descrittivi. Ad esempio, il primecount non è davvero un conteggio di numeri primi. Invece è una variabile indice nel vettore dei numeri primi conosciuti. Inoltre, mentre a volte no viene usato come abbreviazione di "numero", nella scrittura matematica, è più comune usare n . Possiamo anche apportare alcune modifiche eliminando l' break e dichiarando le variabili più vicine al punto in cui vengono utilizzate.

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

Possiamo anche refactoring main usare un "range-for" per renderlo un po 'più ordinato:

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

Questo è solo un modo in cui potrebbe essere fatto il refactoring. Altri potrebbero fare scelte diverse. Tuttavia, lo scopo del refactoring rimane lo stesso, ovvero migliorare la leggibilità e possibilmente le prestazioni del codice senza necessariamente aggiungere funzionalità.

Goto Cleanup

Nelle basi di codice C ++ che erano C, si può trovare il modello goto cleanup . Poiché il comando goto rende il flusso di lavoro di una funzione più difficile da capire, questo è spesso evitato. Spesso, può essere sostituito da dichiarazioni di ritorno, loop, funzioni. Tuttavia, con il goto cleanup necessario eliminare la logica di pulizia.

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

In C ++ si potrebbe usare RAII per risolvere questo problema:

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

Da questo punto in poi, si potrebbe continuare a rifare il codice attuale. Ad esempio, sostituendo VectorRAII con std::unique_ptr o std::vector .