C++
Refactoringstechnieken

Invoering

Refactoring doorloop

Hier is een programma dat baat kan hebben bij refactoring. Het is een eenvoudig programma met C ++ 11 dat bedoeld is om alle priemgetallen van 1 tot 100 te berekenen en af te drukken en is gebaseerd op een programma dat ter beoordeling op CodeReview is geplaatst.

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

De output van dit programma ziet er als volgt uit:

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

Het eerste wat ons opvalt, is dat het programma er niet in slaagt 2 af te drukken, wat een priemgetal is. We kunnen eenvoudig een coderegel toevoegen om die ene constante eenvoudig af te drukken zonder de rest van het programma aan te passen, maar het is misschien netter om het programma te herschikken om het in twee delen te splitsen - een die de priemgetallenlijst maakt en een andere die ze afdrukt . Dit is hoe het eruit zou kunnen zien:

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

Als we deze versie proberen, zien we dat het inderdaad nu correct werkt:

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

De volgende stap is het bericht dat de tweede if clausule is niet echt nodig. De logica in de lus zoekt naar priemfactoren van elk gegeven nummer tot de vierkantswortel van dat nummer. Dit werkt omdat als er priemfactoren van een getal zijn, ten minste één daarvan kleiner moet zijn dan of gelijk aan de vierkantswortel van dat getal. Alleen die functie opnieuw bewerken (de rest van het programma blijft hetzelfde) krijgen we dit resultaat:

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

We kunnen verder gaan en variabelenamen een beetje beschrijvend maken. primecount is bijvoorbeeld niet echt een telling van priemgetallen. In plaats daarvan is het een indexvariabele in de vector van bekende priemgetallen. Hoewel no soms wordt gebruikt als afkorting voor "nummer", is het in wiskundig gebruik gebruikelijker om n te gebruiken. We kunnen ook enkele wijzigingen aanbrengen door de break elimineren, en door variabelen dichter bij waar ze worden gebruikt te verklaren.

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

We kunnen ook main gebruiken om een "bereik-voor" te gebruiken om het een beetje netter te maken:

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

Dit is slechts een manier om refactoring mogelijk te maken. Anderen kunnen verschillende keuzes maken. Het doel voor refactoring blijft echter hetzelfde, namelijk het verbeteren van de leesbaarheid en mogelijk de prestaties van de code zonder noodzakelijkerwijs functies toe te voegen.

Ga naar opruimen

In C ++ codebases die vroeger C waren, kan men het patroon vinden om goto cleanup . Omdat het goto commando de workflow van een functie moeilijker te begrijpen maakt, wordt dit vaak vermeden. Vaak kan het worden vervangen door retourinstructies, lussen, functies. Maar met de goto cleanup moet men zich ontdoen van de opruimlogica.

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 ++ zou je RAII kunnen gebruiken om dit probleem op te lossen:

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

Vanaf dit punt zou men kunnen doorgaan met de refactoring van de daadwerkelijke code. Bijvoorbeeld door de VectorRAII vervangen door std::unique_ptr of std::vector .