Clever Geek Handbook

Penerapan pewarisan di C ++. Kisah nyata

Halo, Habr!



Dalam mencari inspirasi, cara mengisi kembali portofolio penerbit dengan topik C ++, kami menemukan blog Arthur O'Dwyer, yang, omong-omong, telah menulis satu buku tentang C ++, yang muncul entah dari mana . Posting hari ini adalah tentang kode bersih . Kami berharap kasus itu sendiri dan penulisnya menarik bagi Anda.



Semakin saya berurusan dengan kode polimorfik klasik, semakin saya menghargai idiom "modern" yang telah berkembang - idiom antarmuka non-virtual, prinsip substitusi Liskov , aturan Scott Myers bahwa semua kelas harus abstrak atau final, aturan bahwa ada hierarki harus benar-benar dua tingkat, dan aturan bahwa kelas dasar mengekspresikan kesatuan antarmuka , dan tidak menggunakan kembali implementasi.



Hari ini saya ingin menunjukkan kepada Anda cuplikan kode yang mendemonstrasikan bagaimana "warisan implementasi" menyebabkan masalah, dan pola apa yang saya gunakan untuk mengungkapnya. Sayangnya, kode yang membuat saya kesal begitu tidak terbaca sehingga di sini saya memutuskan untuk menunjukkannya dalam bentuk yang disederhanakan dan sedikit khusus domain. Saya akan mencoba menguraikan keseluruhan masalah secara bertahap.



Tahap 1: Transaksi



Misalkan bidang subjek kita adalah "transaksi perbankan". Kami memiliki hierarki antarmuka polimorfik klasik. 



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


Berbagai macam transaksi memiliki API umum tertentu, dan setiap jenis transaksi juga memiliki API spesifiknya sendiri. 



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


Tahap 2: Filter Transaksi



Namun pada kenyataannya, program kami tidak mengeksekusi transaksi, tetapi melacaknya untuk menandai transaksi yang mencurigakan. Operator manusia dapat menyetel filter yang memenuhi kriteria tertentu, seperti "tandai semua transaksi di atas $ 10.000" atau "tandai semua transaksi yang dilakukan atas nama orang di daftar periksa W". Secara internal, kami mewakili berbagai jenis filter yang dapat dikonfigurasi operator sebagai hierarki polimorfik klasik. 



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


Berikut contoh filter sederhana: 



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


Tahap 3: Tersandung Pertama Kali



Ternyata beberapa filter benar-benar mencoba mengakses API tersebut yang khusus untuk transaksi tertentu - API ini telah dibahas di atas. Katakanlah ia DifferentCustomerFiltermencoba menandai transaksi apa pun yang nama pelaksananya berbeda dari nama yang ditentukan dalam faktur. Sebagai contoh, anggaplah bank diatur secara ketat: hanya pemilik akun ini yang dapat menarik uang dari akun. Oleh karena itu, hanya kelas yang DepositTxnmemperhatikan pencatatan nama klien yang melakukan transaksi. 



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


Ini adalah penyalahgunaan klasik dynamic_cast! (Klasik - karena ditemukan sepanjang waktu). Untuk memperbaiki kode ini, seseorang dapat mencoba menerapkan metode dari " Kunjungan polimorfik klasik " (2020-09-29), tetapi, sayangnya, tidak ada perbaikan yang diamati: 



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


Oleh karena itu, penulis kode tersebut menyadari (sarkasme!) Sebuah gagasan. Mari terapkan sensitivitas huruf besar / kecil di beberapa filter. Mari kita tulis ulang kelas dasarnya Filterseperti ini: 



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


Taktik pintar ini mengurangi jumlah kode yang perlu Anda tulis DifferentCustomerFilter. Tapi kami melanggar salah satu prinsip OOP, yaitu larangan pewarisan pelaksanaan. Fungsinya Filter::do_generic(const Txn&)tidak murni atau final. Ini akan kembali menghantui kita.



Langkah 4: Tersandung untuk kedua kalinya



Sekarang mari kita buat filter yang memeriksa apakah pemegang akun ada di daftar hitam negara bagian. Kami ingin menguji beberapa dari daftar ini. 



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, kita perlu membuat filter lain yang menggambar kisi yang lebih luas - filter ini akan memeriksa pemegang akun dan nama pengguna. Sekali lagi, penulis kode asli memiliki ide (sarkasme!) Pintar. Mengapa menduplikasi logika is_flagged, mari lebih baik mewarisinya. Warisan hanyalah penggunaan kembali kode, bukan? 



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


Perhatikan bagaimana arsitektur yang dihasilkan sangat membingungkan. NameWatchlistFiltermenimpa do_genericuntuk hanya memvalidasi nama belakang pemegang rekening, lalu WideNetFiltermenimpanya kembali ke tampilan aslinya. (Jika saya WideNetFiltertidak mendefinisikannya kembali, itu WideNetFilterakan bekerja secara tidak benar untuk setiap transaksi setoran yang name_on_account()tidak ditandai, tetapi name_of_customer()ditandai.) Ini membingungkan, tetapi berfungsi ... untuk saat ini.



Tahap 5: Serangkaian peristiwa yang tidak menyenangkan



Hutang teknis ini menggigit kami ke arah yang tidak terduga, karena kami perlu menambahkan jenis transaksi baru. Mari membuat kelas untuk merepresentasikan pembayaran komisi dan bunga yang dilakukan oleh bank itu sendiri. 



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


Langkah terpenting: kami lupa memperbarui WideNetFilter, menambahkan timpaan untuk WideNetFilter::do_casewise(const FeeTxn&) const. Dalam contoh "mainan" ini, kesalahan seperti itu mungkin langsung tampak tak termaafkan, tetapi dalam kode nyata, di mana dari satu redefiner ke lusinan baris kode lainnya, dan idiom antarmuka non-virtual tidak diikuti dengan cemburu - saya pikir tidak akan sulit menemukan class WideNetFilter : public NameWatchlistFilteryang menimpa seperti do_genericini dan do_casewise, dan berpikir, “Oh, ada sesuatu yang membingungkan di sini. Saya akan kembali ke ini nanti ”(dan tidak pernah kembali ke ini).



Bagaimanapun, saya harap Anda sudah yakin bahwa pada titik ini kita memiliki monster. Bagaimana kita mengecewakannya sekarang?



Refactoring, menyingkirkan warisan implementasi. Langkah 1



Untuk menyingkirkan implementasi inheritance diFilter::do_casewise, mari kita terapkan dalil Scott Myers bahwa metode virtual apa pun harus murni atau final (secara logis). Dalam hal ini, ini adalah kompromi, karena di sini kami melanggar aturan bahwa hierarki harus dangkal. Kami memperkenalkan kelas menengah: 



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 yang menyediakan pemrosesan case-sensitive untuk setiap kemungkinan transaksi sekarang dapat dengan mudah diwarisi dari CasewiseFilter. Filter yang implementasinya generik masih mewarisi langsung dari 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; }
};


Jika seseorang menambahkan tipe transaksi baru dan perubahan CasewiseFilteruntuk memasukkan overload baru do_casewise, maka kita akan melihat bahwa kita telah DifferentCustomerFiltermenjadi kelas abstrak: kita harus berurusan dengan transaksi tipe baru. Sekarang kompilator membantu untuk mematuhi aturan arsitektur kita, yang biasanya digunakan untuk menyembunyikan kesalahan kita secara diam-diam.



Juga perhatikan bahwa sekarang tidak mungkin untuk mendefinisikan WideNetFilteristilah NameWatchlistFilter.



Refactoring, menyingkirkan warisan implementasi. Langkah 2



Untuk menghilangkan pelaksanaan pewarisan dalam WideNetFilter, berlaku prinsip tanggung jawab tunggal . Saat ini, dia sedang NameWatchlistFiltermemecahkan dua masalah: dia bekerja sebagai penyaring penuh dan memiliki kemampuan is_flagged. Mari kita jadikan is_flaggedsebagai kelas WatchlistGroupyang berdiri sendiri yang tidak perlu menyesuaikan dengan API class Filter, karena ini bukan filter, tetapi hanya kelas pembantu yang berguna. 



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


Sekarang WideNetFilterdapat digunakan is_flaggedtanpa mewarisi NameWatchlistFilter. Di kedua filter, Anda dapat mengikuti rekomendasi terkenal dan lebih memilih komposisi daripada warisan, karena komposisi adalah alat untuk menggunakan kembali kode, tetapi warisan tidak. 



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


Sekali lagi, jika seseorang menambahkan jenis transaksi baru dan memodifikasi CasewiseFilteruntuk menyertakan kelebihan muatan baru do_casewise, maka kita akan memastikan untuk WideNetFiltermenjadi kelas abstrak: kita harus berurusan dengan transaksi jenis baru di WideNetFilter(tetapi tidak dalam NameWatchlistFilter). Ini seperti kompilator menyimpan daftar tugas untuk kita!



kesimpulan



Saya telah menganonimkan dan sangat menyederhanakan contoh ini dibandingkan dengan kode yang harus saya gunakan. Tetapi garis besar umum dari hierarki kelas persis seperti itu, seperti logika tipis do_generic(txn) && do_casewise(txn)dari kode aslinya. Saya pikir dari penjelasan di atas tidak begitu mudah untuk memahami betapa bug tersembunyi dalam struktur lama FeeTxn. Sekarang saya telah menyajikan versi yang disederhanakan ini kepada Anda (secara harfiah mengunyahnya untuk Anda!), Saya sendiri secara praktis terkejut, apakah versi asli kode itu sangat buruk? Mungkin tidak ... lagipula, kode ini berfungsi untuk sementara waktu.

Tapi saya harap Anda setuju bahwa versi refactoring, menggunakan CasewiseFilterdan terutama WatchlistGroup, ternyata jauh lebih baik. Jika saya harus memilih yang mana dari dua basis kode ini untuk digunakan, saya tidak akan ragu untuk mengambil yang kedua.


More articles:


All Articles