Clever Geek Handbook

Implementierungsvererbung in C ++. Wahre Begebenheit

Hallo Habr!



Auf der Suche nach Inspiration, wie das Portfolio des Verlages zum Thema C aufzufüllen ++, stießen wir auf das Blog von Arthur O'Dwyer, der, nebenbei bemerkt , schon geschrieben ein Buch über C ++, die aus dem Nichts erschienen aus . Der heutige Beitrag handelt von sauberem Code . Wir hoffen, dass sowohl der Fall selbst als auch der Autor für Sie von Interesse sind.



Je mehr ich mich mit klassischem polymorphem Code beschäftige, desto mehr schätze ich die "modernen" Redewendungen, über die er gewachsen ist - die Redewendung einer nicht virtuellen Schnittstelle, das Liskov-Substitutionsprinzip , Scott Myers 'Regel, dass alle Klassen entweder abstrakt oder endgültig sein müssen, die Regel, die es gibt Die Hierarchie muss streng zweistufig sein und die Regel, dass Basisklassen die Einheit der Schnittstelle ausdrücken und die Implementierung nicht wiederverwenden.



Heute möchte ich Ihnen ein Code-Snippet zeigen, das zeigt, wie "Implementierungsvererbung" zu Problemen führte und welche Muster ich verwendet habe, um es zu entwirren. Leider war der Code, der mich verärgerte, so unleserlich, dass ich mich hier entschied, ihn in einer vereinfachten und leicht themenorientierten Form anzuzeigen. Ich werde versuchen, das ganze Problem schrittweise zu skizzieren.



Stufe 1: Transaktionen



Angenommen, unser Themenbereich ist "Bankgeschäfte". Wir haben eine klassische polymorphe Schnittstellenhierarchie. 



class Txn { ... };
class DepositTxn : public Txn { ... };
class WithdrawalTxn : public Txn { ... };
class TransferTxn : public Txn { ... };


Eine Vielzahl von Transaktionen verfügt über bestimmte gemeinsame APIs, und jeder Transaktionstyp verfügt auch über eine eigene spezifische API. 



class Txn {
public:
    AccountNumber account() const;
    std::string name_on_account() const;
    Money amount() const;
private:
    //  
};

class DepositTxn : public Txn {
public:
    std::string name_of_customer() const;
};

class TransferTxn : public Txn {
public:
    AccountNumber source_account() const;
};


Stufe 2: Transaktionsfilter



In Wirklichkeit führt unser Programm jedoch keine Transaktionen aus, sondern verfolgt sie, um verdächtige Transaktionen zu kennzeichnen. Der menschliche Bediener kann Filter festlegen, die bestimmte Kriterien erfüllen, z. B. "Alle Transaktionen über 10.000 US-Dollar kennzeichnen" oder "Alle Transaktionen kennzeichnen, die im Auftrag von Personen auf der Checkliste W ausgeführt werden". Intern stellen wir die verschiedenen Arten von vom Bediener konfigurierbaren Filtern als klassische polymorphe Hierarchie dar. 



class Filter { ... };
class AmountGtFilter : public Filter { ... };
class NameWatchlistFilter : public Filter { ... };
class AccountWatchlistFilter : public Filter { ... };
class DifferentCustomerFilter : public Filter { ... };
class AndFilter : public Filter { ... };
class OrFilter : public Filter { ... };
         API.
class Filter {
public:
    bool matches(const Txn& txn) const {
        return do_matches(txn);
    }
private:
    virtual bool do_matches(const Txn&) const = 0;
};


Hier ist ein Beispiel für einen einfachen Filter: 



class AmountGtFilter : public Filter {
public:
    explicit AmountGtFilter(Money x) : amount_(x) { }
private:
    bool do_matches(const Txn& txn) const override {
        return txn.amount() > amount_;
    }

    Money amount_;
};


Stufe 3: Das erste Mal gestolpert



Es stellt sich heraus, dass einige Filter tatsächlich versuchen, auf die APIs zuzugreifen, die für bestimmte Transaktionen spezifisch sind. Diese APIs wurden oben erläutert. Angenommen, es wird DifferentCustomerFilterversucht, eine Transaktion zu kennzeichnen, bei der sich der Name des Ausführenden von dem in der Rechnung angegebenen Namen unterscheidet. Nehmen wir zum Beispiel an, dass die Bank streng reguliert ist: Nur der Inhaber dieses Kontos kann Geld von einem Konto abheben. Daher ist nur die Klasse DepositTxnbesorgt, den Namen des Clients aufzuzeichnen, der die Transaktion durchgeführt hat. 



class DifferentCustomerFilter : public Filter {
    bool do_matches(const Txn& txn) const override {
        if (auto *dtxn = dynamic_cast<const DepositTxn*>(&txn)) {
            return dtxn->name_of_customer() != dtxn->name_on_account();
        } else {
            return false;
        }
    }
};


Dies ist ein klassischer Missbrauch von dynamic_cast! (Klassisch - weil es die ganze Zeit gefunden wird). Um diesen Code zu beheben, könnte man versuchen, die Methode aus " Klassisch polymorpher Besuch " (29.09.2020) anzuwenden, aber leider wurden keine Verbesserungen beobachtet: 



class DifferentCustomerFilter : public Filter {
    bool do_matches(const Txn& txn) const override {
        my::visit<DepositTxn>(txn, [](const auto& dtxn) {
            return dtxn.name_of_customer() != dtxn.name_on_account();
        }, [](const auto&) {
            return false;
        });
    }
};


Daher wurde der Autor des Codes (Sarkasmus!) Mit einer Idee geschlagen. Lassen Sie uns die Groß- und Kleinschreibung in einigen Filtern implementieren. Schreiben wir die Basisklasse folgendermaßen um Filter: 



class Filter {
public:
    bool matches(const Txn& txn) const {
        return my::visit<DepositTxn, WithdrawalTxn, TransferTxn>(txn, [](const auto& txn) {
            return do_generic(txn) && do_casewise(txn);
        });
    }
private:
    virtual bool do_generic(const Txn&) const { return true; }
    virtual bool do_casewise(const DepositTxn&) const { return true; }
    virtual bool do_casewise(const WithdrawalTxn&) const { return true; }
    virtual bool do_casewise(const TransferTxn&) const { return true; }
};

class LargeAmountFilter : public Filter {
    bool do_generic(const Txn& txn) const override {
        return txn.amount() > Money::from_dollars(10'000);
    }
};

class DifferentCustomerFilter : public Filter {
    bool do_casewise(const DepositTxn& dtxn) const override {
        return dtxn.name_of_customer() != dtxn.name_on_account();
    }
    bool do_casewise(const WithdrawalTxn&) const override { return false; }
    bool do_casewise(const TransferTxn&) const override { return false; }
};


Diese clevere Taktik reduziert die Menge an Code, die Sie schreiben müssen DifferentCustomerFilter. Wir verstoßen jedoch gegen eines der Prinzipien der OOP, nämlich das Verbot der Vererbung der Umsetzung. Die Funktion ist Filter::do_generic(const Txn&)weder rein noch endgültig. Dies wird zurückkommen, um uns zu verfolgen.



Schritt 4: Ein zweites Mal gestolpert



Lassen Sie uns nun einen Filter erstellen , der prüft, ob der Kontoinhaber auf einer staatlichen schwarzen Liste steht. Wir möchten einige dieser Listen testen. 



class NameWatchlistFilter : public Filter {
protected:
    bool is_flagged(std::string_view name) const {
        for (const auto& list : watchlists_) {
            if (std::find(list.begin(), list.end(), name) != list.end()) {
                return true;
            }
        }
        return false;
    }

private:
    bool do_generic(const Txn& txn) const override {
        return is_flagged(txn.name_on_account());
    }

    std::vector<std::list<std::string>> watchlists_;
};


Oh, wir müssen einen weiteren Filter erstellen, der ein breiteres Raster zeichnet - er überprüft sowohl den Kontoinhaber als auch den Benutzernamen. Auch hier hat der Autor des Originalcodes eine (Sarkasmus!) Kluge Idee. Warum doppelte Logik is_flagged, lasst sie uns besser erben. Vererbung ist nur eine Wiederverwendung von Code, oder? 



class WideNetFilter : public NameWatchlistFilter {
    bool do_generic(const Txn& txn) const override {
        return true;
    }
    bool do_casewise(const DepositTxn& txn) const override {
        return is_flagged(txn.name_on_account()) || is_flagged(txn.name_of_customer());
    }
    bool do_casewise(const WithdrawalTxn& txn) const override {
        return is_flagged(txn.name_on_account());
    }
    bool do_casewise(const TransferTxn& txn) const override {
        return is_flagged(txn.name_on_account());
    }
};


Beachten Sie, wie schrecklich die resultierende Architektur verwirrt ist. NameWatchlistFilterüberschreibt, do_genericum nur den Nachnamen des Kontoinhabers zu überprüfen, und WideNetFilterüberschreibt ihn dann wieder in seiner ursprünglichen Ansicht. (Wenn ich es WideNetFilternicht zurück definiert hätte, WideNetFilterhätte es bei jeder Einzahlungstransaktion, bei der es name_on_account()nicht markiert, sondern name_of_customer()markiert ist, falsch funktioniert .) Das ist verwirrend, aber es funktioniert ... für jetzt.



Stufe 5: Eine Reihe unangenehmer Ereignisse



Diese technische Verschuldung hat uns in eine so unerwartete Richtung gebissen, dass wir eine neue Art von Transaktion hinzufügen mussten. Lassen Sie uns eine Klasse bilden, um Provisionen und Zinszahlungen darzustellen, die von der Bank selbst initiiert wurden. 



class FeeTxn : public Txn { ... };

class Filter {
public:
    bool matches(const Txn& txn) const {
        return my::visit<DepositTxn, WithdrawalTxn, TransferTxn, FeeTxn>(txn, [](const auto& txn) {
            return do_generic(txn) && do_casewise(txn);
        });
    }
private:
    virtual bool do_generic(const Txn&) const { return true; }
    virtual bool do_casewise(const DepositTxn&) const { return true; }
    virtual bool do_casewise(const WithdrawalTxn&) const { return true; }
    virtual bool do_casewise(const TransferTxn&) const { return true; }
    virtual bool do_casewise(const FeeTxn&) const { return true; }
};


Der wichtigste Schritt: Wir haben vergessen zu aktualisieren WideNetFilter, eine Überschreibung für hinzuzufügen WideNetFilter::do_casewise(const FeeTxn&) const. In diesem „Spielzeug“ Beispiel ein solcher Fehler kann erscheinen sofort unverzeihlich, aber in echten Code, wo von einem redefiner auf einen anderen Dutzende von Zeilen Code, und das Idiom einer nicht-virtuellen Schnittstelle ist nicht sehr sorgfältig beobachtet - ich denke , es wird nicht schwierig sein , einen zu finden , class WideNetFilter : public NameWatchlistFilterdass Überschreibungen wie do_genericdiese und do_casewiseund denke: „Oh, hier ist etwas verwirrend. Ich werde später darauf zurückkommen “(und nie wieder darauf zurückkommen).



Auf jeden Fall hoffe ich, dass Sie bereits davon überzeugt sind, dass wir zu diesem Zeitpunkt ein Monster haben. Wie entzaubern wir ihn jetzt?



Refactoring, Beseitigung der Implementierungsvererbung. Schritt 1



Entfernen der Implementierungsvererbung inFilter::do_casewiseWenden Sie das Postulat von Scott Myers an, dass jede virtuelle Methode entweder rein oder (logisch) endgültig sein muss. In diesem Fall ist dies ein Kompromiss, da wir hier die Regel brechen, dass Hierarchien flach sein sollten. Wir stellen eine Zwischenklasse vor: 



class Filter {
public:
    bool matches(const Txn& txn) const {
        return do_generic(txn);
    }
private:
    virtual bool do_generic(const Txn&) const = 0;
};

class CasewiseFilter : public Filter {
    bool do_generic(const Txn&) const final {
        return my::visit<DepositTxn, WithdrawalTxn, TransferTxn>(txn, [](const auto& txn) {
            return do_casewise(txn);
        });
    }

    virtual bool do_casewise(const DepositTxn&) const = 0;
    virtual bool do_casewise(const WithdrawalTxn&) const = 0;
    virtual bool do_casewise(const TransferTxn&) const = 0;
};


Filter, die bei jeder möglichen Transaktion zwischen Groß- und Kleinschreibung unterscheiden, können jetzt einfach von erben CasewiseFilter. Filter, deren Implementierungen generisch sind, erben immer noch direkt von Filter. 



class LargeAmountFilter : public Filter {
    bool do_generic(const Txn& txn) const override {
        return txn.amount() > Money::from_dollars(10'000);
    }
};

class DifferentCustomerFilter : public CasewiseFilter {
    bool do_casewise(const DepositTxn& dtxn) const override {
        return dtxn.name_of_customer() != dtxn.name_on_account();
    }
    bool do_casewise(const WithdrawalTxn&) const override { return false; }
    bool do_casewise(const TransferTxn&) const override { return false; }
};


Wenn jemand einen neuen Transaktionstyp hinzufügt und Änderungen hinzufügt CasewiseFilter, um eine neue Überladung aufzunehmen do_casewise, werden wir feststellen, dass wir DifferentCustomerFilterzu einer abstrakten Klasse geworden sind: Wir müssen uns mit einer Transaktion eines neuen Typs befassen. Jetzt hilft der Compiler dabei, die Regeln unserer Architektur einzuhalten, wo er unsere Fehler stillschweigend versteckte.



Beachten Sie auch, dass es jetzt unmöglich ist, WideNetFilterBegriffe zu definieren NameWatchlistFilter.



Refactoring, Beseitigung der Implementierungsvererbung. Schritt 2



Um die Vererbung der Implementierung zu beseitigen WideNetFilter, gilt das Prinzip der alleinigen Verantwortung . Im Moment NameWatchlistFilterlöst er zwei Probleme: Er arbeitet als vollwertiger Filter und hat die Fähigkeit is_flagged. Machen wir es zu is_flaggedeiner eigenständigen Klasse WatchlistGroup, die nicht der API entsprechen muss class Filter, da es sich nicht um einen Filter handelt, sondern nur um eine nützliche Hilfsklasse. 



class WatchlistGroup {
public:
    bool is_flagged(std::string_view name) const {
        for (const auto& list : watchlists_) {
            if (std::find(list.begin(), list.end(), name) != list.end()) {
                return true;
            }
        }
        return false;
    }
private:
    std::vector<std::list<std::string>> watchlists_;
};


WideNetFilterKann jetzt is_flaggedohne Erben verwenden NameWatchlistFilter. In beiden Filtern können Sie der bekannten Empfehlung folgen und die Komposition der Vererbung vorziehen, da die Komposition ein Werkzeug zur Wiederverwendung von Code ist, die Vererbung jedoch nicht. 



class NameWatchlistFilter : public Filter {
    bool do_generic(const Txn& txn) const override {
        return wg_.is_flagged(txn.name_on_account());
    }

    WatchlistGroup wg_;
};

class WideNetFilter : public CasewiseFilter {
    bool do_casewise(const DepositTxn& txn) const override {
        return wg_.is_flagged(txn.name_on_account()) || wg_.is_flagged(txn.name_of_customer());
    }
    bool do_casewise(const WithdrawalTxn& txn) const override {
        return wg_.is_flagged(txn.name_on_account());
    }
    bool do_casewise(const TransferTxn& txn) const override {
        return wg_.is_flagged(txn.name_on_account());
    }

    WatchlistGroup wg_;
};


Wenn jemand einen neuen Transaktionstyp hinzufügt und ihn so ändert CasewiseFilter, dass er eine neue Überladung enthält do_casewise, stellen wir sicher, WideNetFilterdass wir eine abstrakte Klasse werden: Wir müssen uns mit Transaktionen eines neuen Typs in WideNetFilter(aber nicht in NameWatchlistFilter) befassen . Es ist, als ob der Compiler eine To-Do-Liste für uns führt!



Schlussfolgerungen



Ich habe dieses Beispiel im Vergleich zu dem Code, mit dem ich arbeiten musste, anonymisiert und extrem vereinfacht. Aber der allgemeine Umriss der Klassenhierarchie war genau das, ebenso wie die schwache Logik do_generic(txn) && do_casewise(txn)aus dem ursprünglichen Code. Ich denke, dass es von oben nicht so einfach ist zu verstehen, wie unmerklich der Fehler in der alten Struktur versteckt war FeeTxn. Nachdem ich Ihnen diese vereinfachte Version vorgestellt habe (buchstäblich für Sie zerkaut!), Bin ich selbst praktisch überrascht, war die Originalversion des Codes so schlecht? Vielleicht nicht ... schließlich funktionierte dieser Code eine Weile.

Aber ich hoffe, Sie werden mir zustimmen, dass die Refactoring-Version mit CasewiseFilterund insbesondere WatchlistGroupviel besser geworden ist. Wenn ich mich für eine dieser beiden Codebasen entscheiden müsste, würde ich ohne zu zögern die zweite nehmen.


More articles:


All Articles