C++
리팩토링 기법

소개

리팩토링

리팩토링의 이점을 누릴 수있는 프로그램이 있습니다. 이것은 C ++ 11을 사용하는 간단한 프로그램입니다.이 프로그램은 1에서 100까지의 모든 소수를 계산하고 인쇄하며, 검토를 위해 CodeReview 에 게시 된 프로그램을 기반으로합니다.

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

이 프로그램의 출력은 다음과 같습니다.

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

우리가주의해야 할 첫 번째 점은 프로그램이 소수 2 인 2 를 인쇄하지 못한다는 것입니다. 프로그램의 나머지 부분을 수정하지 않고 그 하나의 상수를 간단하게 인쇄하기위한 코드 행을 추가 할 수는 있지만, 프로그램을 두 부분으로 나눌 리팩터링 하는 것이 더 좋을 수 있습니다 - 소수 목록을 만드는 프로그램과 소수를 만드는 프로그램을 인쇄하는 다른 프로그램 . 다음과 같이 표시 될 수 있습니다.

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

이 버전을 사용하면 실제로 올바르게 작동하는 것을 볼 수 있습니다.

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

다음 단계는 두 번째 if 절이 실제로 필요 없다는 것을 알아 차리는 것입니다. 루프의 논리는 주어진 숫자의 제곱근을 그 숫자의 제곱근까지 찾습니다. 이는 숫자의 소수 요소가있을 경우 그 중 적어도 하나가 해당 숫자의 제곱근보다 작거나 같아야하기 때문에 효과가 있습니다. 이 함수를 다시 작성하면 (나머지 프로그램은 동일하게 유지됩니다)이 결과를 얻습니다.

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

변수 이름을 조금 더 설명 적으로 변경하면 더 나아갈 수 있습니다. 예를 들어 primecount 는 실제로 소수의 수는 아닙니다. 대신 알려진 소수의 벡터에 대한 인덱스 변수입니다. 또한 no 는 "number"의 약어로 사용되기도하지만 수학 서적에서는 n 을 사용하는 것이 더 일반적입니다. 또한 break 을 없애고 변수를 사용 위치에 가깝게 선언함으로써 일부 수정을 할 수 있습니다.

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

우리는 또한 "range-for"를 사용하여 main 을 리팩터링하여 조금 더 깔끔하게 만들 수 있습니다 :

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

이는 리팩토링이 수행 될 수있는 한 가지 방법 일뿐입니다. 다른 사람들은 다른 선택을 할 수 있습니다. 그러나 리팩토링의 목적은 그대로 유지됩니다. 이는 기능을 추가하지 않아도 코드의 가독성과 성능을 향상시키는 것입니다.

고토 정리

C로 사용되는 C ++ 코드 기반에서는 goto cleanup 패턴을 찾을 수 있습니다. goto 명령은 함수의 워크 플로를 이해하기 어렵게 만들기 때문에 종종 피하는 것이 좋습니다. 흔히 return 문, 루프, 함수로 대체 될 수 있습니다. 하지만 goto cleanup 로 정리 논리를 제거해야합니다.

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

C ++에서 RAII 를 사용하여이 문제를 해결할 수 있습니다.

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

이 시점부터 실제 코드를 리팩터링 할 수 있습니다. 예를 들어 VectorRAII 를 std::unique_ptr 또는 std::vector VectorRAII .