2015-03-13 10 views
14

Mam projekt klasy podobny do następującego:funkcje państwa, które są czasami const

class MyClass { 
public: 
    bool IsValid() const; 
    void MakeValid(); 
private: 
    bool CheckValidity(bool fix); 
}; 

bool MyClass::IsValid() const { 
    // Check validity, but don't fix any problems found. Doesn't work. 
    return CheckValidity(false); 
} 

void MyClass::MakeValid() { 
    // Check validity and fix problems found. 
    CheckValidity(true); 
} 

IsValid powinny być const, ponieważ nie dokonać zmian. MakeValid nie powinien być stały, ponieważ wprowadza zmiany. Dzielą one tę samą implementację, CheckValidity, ale ponieważ CheckValidity może lub nie może wprowadzać zmian, nie można oznaczyć go jako const.

Jaki jest najlepszy sposób, aby sobie z tym poradzić? Najprostszym sposobem jest po prostu użyć const_cast, ale odlewania z dala const czuje się trochę brudna:

bool MyClass::IsValid() const { 
    // Check validity, but don't fix any problems found. 
    return const_cast<MyClass*>(this)->CheckValidity(false); 
} 

Jest to uzasadnione wykorzystanie const_cast? Czy istnieje lepsze podejście?

+14

Dzielenie czeków i "ustalanie" na dwie różne funkcje? Kontrola to 'const', a poprawka nie jest? – crashmstr

+6

Zgadzam się z @crashmstr, mając jedną funkcję, która ma dwie różne rzeczy, to zły zapach. –

+5

W rzeczywistości już nazwa 'CheckValidity' sugeruje, że ta funkcja sprawdza tylko i może być stała. Jeśli chcesz 'MakeValid' to jest coś innego (i nie powinno być const ...) – user463035818

Odpowiedz

16

Jestem zakładając implementacji wygląda podobnie do tego:

bool CheckValidity(bool fix) 
{ 
    // Actually check validity. 
    bool isValid = ...; 

    if (!isValid && fix) 
    { 
     // Attempt to fix validity (and update isValid). 
     isValid = ...; 
    } 

    return isValid; 
} 

naprawdę masz dwie różne funkcje popchnął w jednym. Jednym z kluczowych wskaźników tego rodzaju uwikłania jest boolowski argument funkcji, która pachnie, ponieważ osoba dzwoniąca nie może od razu odróżnić, czy ustawić wartość true lub false bez odwoływania się do kodu/dokumentów.

Podziel się metodę:

bool CheckValidity() const 
{ 
    // Actually check validity. 
    bool isValid = ...; 
    return isValid; 
} 

void FixValidity() 
{ 
    // Attempt to fix validity. 
    // ... 
} 

A potem twoje metody publiczne mogą dokonać połączenia bardziej odpowiednio.

bool IsValid() const 
{ 
    // No problem: const method calling const method 
    return CheckValidity(); 
} 

void MakeValid() 
{ 
    if (!CheckValidity()) // No problem: non-const calling const 
    { 
     FixValidity(); // No problem: non-const calling non-const 
    } 
} 
+1

W tym przypadku logika sprawdzania jest skomplikowana (pętla nad zagnieżdżonymi podstrukturami i sprawdzanie ich pod kątem spójności względem siebie), a rozwiązywanie problemów wymaga znajomości, które podelementy mają problemy, więc staram się znaleźć czysty sposób na oddzielić 'CheckValidity' i' FixValidity' bez powielania lub znacznego komplikowania logiki. –

+5

@JoshKelley Możliwe, że możesz oddzielić przejście od sprawdzania od manipulacji. Lub możesz znaleźć Check vs Fix trudne do oddzielenia, ponieważ istniejący kod/struktury są skompilowane. Jednym "łatwym" sposobem na podział i sprawdzanie jest po prostu skopiowanie obecnej metody CheckValidity (bool). W pierwszym przypadku przyjmijmy, że w tej metodzie bool jest fałszywy, usuń martwy kod, a następnie usuń parametr bool. Z drugim zmień nazwę na FixValidity, załóżmy, że bool ma wartość true, usuń martwy kod, a następnie usuń parametr bool. Następnie zobacz, co jest nadal powszechne między tymi dwoma i refaktorami. –

+0

Myśląc o tym dalej, podział ruchu wydaje się być właściwą drogą. Dziękuję Ci. –

6

Oto podejście, które może być przydatne w niektórych przypadkach. Może to być przesada w twojej konkretnej sytuacji.

Twoja funkcja CheckValidity może zostać przekazana obiekt obsługi. Funkcja CheckValidity znajdzie to, co nie było poprawne, i wywoła odpowiednią metodę obiektu obsługi. Możesz mieć wiele różnych metod dla różnych rodzajów naruszeń zasad ważności, a metody te mogą zawierać wystarczającą ilość informacji, aby problem mógł zostać rozwiązany, jeśli to konieczne. Aby zaimplementować IsValid, wystarczy przekazać procedurę obsługi, która ustawia flagę wskazującą, że wystąpił problem. Aby zaimplementować MakeValid, możesz przekazać moduł obsługi, który faktycznie rozwiązuje problem. Problem związany z const rozwiązany jest przez to, że program obsługi utrwalania nie odwołuje się do obiektu.

Oto przykład:

class MyClass { 
public: 
    bool IsValid() const 
    { 
     bool flag = false; 
     CheckValidity(FlagProblems{flag}); 
     return flag; 
    } 

    void MakeValid() 
    { 
     CheckValidity(FixProblems{*this}); 
    } 

private: 
    struct FlagProblems { 
     bool& flag; 

     void handleType1(arg1,arg2)  const { flag = true; } 
     void handleType2(arg1,arg2,arg3) const { flag = true; } 
     . 
     . 
     . 
    }; 

    struct FixProblems { 
     MyClass& object; 
     void handleType1(arg1,arg2)  const { ... } 
     void handleType2(arg1,arg2,arg3) const { ... } 
     . 
     . 
     . 
    }; 

    template <typename Handler> 
    bool CheckValidity(const Handler &handler) const 
    { 
     // for each possible problem: 
     // if it is a type-1 problem: 
     //  handler.handleType1(arg1,arg2); 
     // if it is a type-2 problem: 
     //  handler.handleType2(arg1,arg2,arg3); 
     // . 
     // . 
     // . 
    } 
}; 

Korzystanie z szablonu pozwala na maksymalną wydajność. Alternatywnie użycie klasy bazowej z funkcjami wirtualnymi dla programu obsługi może zapewnić mniejszy rozmiar pliku wykonywalnego.

Jeśli sposoby, w które obiekt może być nieważny, są prostsze, wówczas uzyskanie zwrotu CheckValidity może spowodować, że struktura zawierająca istotne informacje będzie bardziej prosta.

+0

Zdecydowanie ważna możliwość, choć z takim rodzajem separacji, "CheckValidity" może być bardziej odpowiednio nazwany 'Traverse' lub coś podobnego, co oznacza, że ​​porusza się po danych i stosuje procedury obsługi, zamiast wykonywać własne sprawdzanie. –

+1

@MatthewMoss: Naprawdę chciałem powiedzieć, że przechodzimy przez każdy możliwy problem, a nie każdy możliwy obiekt w strukturze danych. Przeredaguję to. –

+0

Ah, zrozumiano. –

2

Możesz użyć specjalizacji szablonu, aby oddzielić części, które mają tylko cel, na obiekcie nie będącym stałym.

Poniżej znajduje się implementacja klasy zabawki. Ma jeden składnik tablicy c v z 10 intami, i dla naszych celów jest ważny tylko wtedy, gdy każdy z nich jest równy zeru.

class ten_zeroes { 
    int v[10]; 
    void fix(int pos) {v[pos] = 0;} 

    public: 
    ten_zeroes() { // construct as invalid object 
    for (int i=0;i<10;i++) { 
     v[i] = i; 
    } 
    } 
}; 

Zobacz, że ja już element funkcyjny, który rozwiązuje nieprawidłową pozycję, i piękny konstruktor, który inicjuje go jako niepoprawnego obiektu (nie rób tego: D)

Ponieważ mamy zamiar użyj szablonów, musimy przenieść implementację cyklu sprawdzania/poprawiania poza klasę. Aby odpowiednie funkcje mogły uzyskać dostęp do metody v i fix(), zrobimy z nich znajomych. Nasz kod teraz wygląda:

class ten_zeroes { 
    int v[10]; 
    void fix(int pos) {v[pos] = 0;} 

    public: 
    ten_zeroes() { // construct as invalid object 
    for (int i=0;i<10;i++) { 
     v[i] = i; 
    } 
    } 

    template<typename T> 
    friend void fix(T& obj, int pos); 

    template<typename T> 
    friend bool check(T& obj); 
}; 

check() „s implementacja jest prosta:

// Check and maybe fix object 
template<typename T> 
bool check(T& obj){ 
    bool result = true; 
    for(int i=0;i<10;i++) { 
    if (obj.v[i]) { 
     result = false; 
     fix(obj, i); 
    } 
    } 
    return result; 
} 

Teraz tutaj jest skomplikowana część. Chcemy, aby nasza funkcja fix() zmieniała zachowanie w oparciu o constness. W tym celu musimy specjalizować szablon. Dla obiektu niestanowiącego stałego, naprawi on pozycję. Dla jednego const, to zrobi nic:

// For a regular object, fix the position 
template<typename T> 
void fix(T& obj, int pos) { obj.fix(pos);} 

// For a const object, do nothing 
template<typename T> 
void fix(const T& obj, int pos) {} 

Wreszcie piszemy nasze is_valid() i make_valid() metody, a tutaj mamy pełne wdrożenie:

#include <iostream> 

class ten_zeroes { 
    int v[10]; 
    void fix(int pos) {v[pos] = 0;} 

    public: 
    ten_zeroes() { // construct as invalid object 
    for (int i=0;i<10;i++) { 
     v[i] = i; 
    } 
    } 

    bool is_valid() const {return check(*this);} // since this is const, it will run check with a const ten_zeroes object 
    void make_valid() { check(*this);} // since this is non-const , it run check with a non-const ten_zeroes object 

    template<typename T> 
    friend void fix(T& obj, int pos); 

    template<typename T> 
    friend bool check(T& obj); 
}; 

// For a regular object, fix the position 
template<typename T> 
void fix(T& obj, int pos) { obj.fix(pos);} 

// For a const object, do nothing 
template<typename T> 
void fix(const T& obj, int pos) {} 

// Check and maybe fix object 
template<typename T> 
bool check(T& obj){ 
    bool result = true; 
    for(int i=0;i<10;i++) { 
    if (obj.v[i]) { 
     result = false; 
     fix(obj, i); 
    } 
    } 
    return result; 
} 

int main(){ 
    ten_zeroes a; 
    std::cout << a.is_valid() << a.is_valid(); // twice to make sure the first one didn't make any changes 
    a.make_valid(); // fix the object 
    std::cout << a.is_valid() << std::endl; // check again 
} 

Mam nadzieję, że nie przeszkadza main() funkcja tam. Będzie testować naszą małą zabawkę, a wyjście 001, zgodnie z oczekiwaniami. Teraz wszelkie czynności związane z tym kodem nie będą musiały zajmować się powielaniem kodu, czego prawdopodobnie chciałeś uniknąć. Mam nadzieję, że to było pomocne.

Oczywiście, jeśli chcesz ukryć te szczegóły implementacji od końcowego użytkownika, powinieneś przenieść je do odpowiedniej szczegółowej przestrzeni nazw. Zostawię to tobie :)

Powiązane problemy