Suche…


Einführung

Refactoring bezieht sich auf die Änderung von vorhandenem Code in eine verbesserte Version. Obwohl Refactoring häufig durchgeführt wird, während Code geändert wird, um Features hinzuzufügen oder Fehler zu beheben, bezieht sich der Begriff insbesondere auf die Verbesserung von Code, ohne notwendigerweise Features hinzuzufügen oder Fehler zu beheben.

Refactoring durchlaufen

Hier ist ein Programm, das vom Refactoring profitieren könnte. Es ist ein einfaches Programm unter Verwendung von C ++ 11, das alle Primzahlen von 1 bis 100 berechnen und drucken soll und auf einem Programm basiert, das zur Überprüfung in CodeReview veröffentlicht wurde .

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

Die Ausgabe dieses Programms sieht folgendermaßen aus:

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

Das erste, was wir bemerken, ist, dass das Programm 2 nicht druckt, was eine Primzahl ist. Wir könnten einfach eine Zeile Code fügen Sie einfach ohne Änderung der Rest des Programms , dass eine Konstante zu drucken, aber es könnte sauberer sein , das Programm Refactoring es in zwei Teile zu spalten - eine , die die Primzahl - Liste eine andere schafft , die sie druckt . So könnte das aussehen:

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

Beim Versuch dieser Version sehen wir, dass sie jetzt richtig funktioniert:

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

Der nächste Schritt ist zu bemerken, dass die zweite if Klausel nicht wirklich benötigt wird. Die Logik in der Schleife sucht nach Primfaktoren von jeder gegebenen Zahl bis zur Quadratwurzel dieser Zahl. Dies funktioniert, weil bei Primfaktoren einer Zahl mindestens einer von ihnen kleiner oder gleich der Quadratwurzel dieser Zahl sein muss. Wenn Sie nur diese Funktion überarbeiten (der Rest des Programms bleibt gleich), erhalten Sie dieses Ergebnis:

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

Wir können noch weiter gehen und Variablennamen ändern, um sie etwas beschreibender zu gestalten. Zum Beispiel ist primecount nicht wirklich eine Anzahl von Primzahlen. Stattdessen handelt es sich um eine Indexvariable in den Vektor bekannter Primzahlen. Auch wenn no manchmal als Abkürzung für "number" verwendet wird, ist es beim mathematischen Schreiben üblicher, n zu verwenden. Wir können auch einige Modifikationen vornehmen, indem wir die break beseitigen und Variablen näher am Verwendungsort deklarieren.

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

Wir können main auch umgestalten, um ein "Range-for" zu verwenden, um es etwas netter zu machen:

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

Dies ist nur eine Möglichkeit, Refactoring durchzuführen. Andere können andere Entscheidungen treffen. Der Zweck des Refactorings bleibt jedoch derselbe, dh die Lesbarkeit und möglicherweise die Leistung des Codes müssen verbessert werden, ohne dass Features hinzugefügt werden müssen.

Gehe zu Bereinigung

In C ++ - Codebasen, die früher C waren, kann man das Muster goto cleanup . Da der Befehl goto den Workflow einer Funktion schwieriger zu verstehen macht, wird dies häufig vermieden. Häufig kann es durch return-Anweisungen, Schleifen und Funktionen ersetzt werden. Allerdings muss man mit der goto cleanup die goto cleanup loswerden.

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 ++ kann man RAII verwenden , um dieses Problem zu beheben:

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

Ab diesem Zeitpunkt könnte der eigentliche Code weiter überarbeitet werden. Zum Beispiel durch Ersetzen des VectorRAII durch std::unique_ptr oder std::vector .



Modified text is an extract of the original Stack Overflow Documentation
Lizenziert unter CC BY-SA 3.0
Nicht angeschlossen an Stack Overflow