Clever Geek Handbook

Herencia de implementación en C ++. Historia real

¡Hola, Habr!



En busca de inspiración, cómo reponer el portafolio de la editorial sobre el tema de C ++, nos encontramos con el blog de Arthur O'Dwyer, quien, por cierto, ya había escrito un libro sobre C ++, que apareció de la nada . La publicación de hoy trata sobre el código limpio . Esperamos que tanto el caso en sí como el autor sean de su interés.



Cuanto más trato con el código polimórfico clásico, más aprecio los modismos "modernos" sobre los que ha crecido: el idioma de una interfaz no virtual, el principio de sustitución de Liskov , la regla de Scott Myers de que todas las clases deben ser abstractas o finales, la regla de que cualquier la jerarquía debe ser estrictamente de dos niveles y la regla de que las clases base expresan la unidad de la interfaz y no reutilizan la implementación.



Hoy quiero mostrarles un fragmento de código que demuestra cómo la "herencia de implementación" generó problemas y qué patrones usé para desentrañarlo. Desafortunadamente, el código que me molestó era tan ilegible que aquí decidí mostrarlo de forma simplificada y ligeramente específica de dominio. Intentaré esbozar todo el problema por etapas.



Etapa 1: Transacciones



Digamos que nuestra área temática es "transacciones bancarias". Tenemos una jerarquía de interfaz polimórfica clásica. 



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


Una amplia variedad de transacciones tienen ciertas API comunes, y cada tipo de transacción también tiene su propia API específica. 



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


Etapa 2: Filtros de transacciones



Pero, en realidad, nuestro programa no ejecuta transacciones, sino que las rastrea para marcar transacciones sospechosas. El operador humano puede establecer filtros que cumplan con ciertos criterios, como "marcar todas las transacciones de más de $ 10,000" o "marcar todas las transacciones realizadas en nombre de las personas en la lista de verificación W". Internamente, representamos los diferentes tipos de filtros configurables por el operador como una jerarquía polimórfica clásica. 



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


A continuación, se muestra un ejemplo de un filtro simple: 



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


Etapa 3: Tropecé por primera vez



Resulta que algunos filtros intentan acceder a las API que son específicas para transacciones específicas; estas API se discutieron anteriormente. Digamos que está DifferentCustomerFilterintentando etiquetar cualquier transacción en la que el nombre de su ejecutor sea diferente del nombre especificado en la factura. A modo de ejemplo, supongamos que el banco está estrictamente regulado: solo el propietario de esta cuenta puede retirar dinero de una cuenta. Por lo tanto, solo la clase se DepositTxnpreocupa por registrar el nombre del cliente que realizó la transacción. 



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


¡Este es un abuso clásico de dynamic_cast! (Clásico, porque se encuentra todo el tiempo). Para corregir este código, se podría intentar aplicar el método de " Visita clásicamente polimórfica " (2020-09-29), pero, lamentablemente, no se han observado mejoras: 



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


Por lo tanto, el autor del código amaneció (¡sarcasmo!) Una idea. Implementemos la distinción entre mayúsculas y minúsculas en algunos filtros. Reescribamos la clase base Filterasí: 



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


Esta inteligente táctica reduce la cantidad de código que necesita escribir DifferentCustomerFilter. Pero estamos violando uno de los principios de la programación orientada a objetos, a saber, la prohibición de la herencia de la implementación. La función Filter::do_generic(const Txn&)no es pura ni final. Esto volverá a perseguirnos.



Paso 4: Tropecé por segunda vez



Ahora hagamos un filtro que verifique si el titular de la cuenta está en alguna lista negra estatal. Queremos probar varias de estas listas. 



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, necesitamos hacer otro filtro que dibuje una cuadrícula más amplia: verificará tanto al titular de la cuenta como al nombre de usuario. Una vez más, el autor del código original tiene una (¡sarcasmo!) Idea inteligente. Por qué duplicar la lógica is_flagged, mejor vamos a heredarla. La herencia es solo una reutilización de código, ¿verdad? 



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


Observe cómo la arquitectura resultante está terriblemente confusa. NameWatchlistFilteranula do_genericsolo para validar el apellido del titular de la cuenta, luego lo WideNetFilteranula a su vista original. (Si no lo hubiera WideNetFilterredefinido de nuevo, se WideNetFilterhabría activado incorrectamente para cualquier transacción de depósito donde no esté name_on_account()marcada, pero sí name_of_customer()marcada). Esto es confuso, pero funciona ... por ahora.



Etapa 5: una serie de eventos desagradables



Esta deuda técnica nos mordió en una dirección tan inesperada, ya que necesitábamos agregar un nuevo tipo de transacción. Hagamos una clase para representar los pagos de comisiones e intereses iniciados por el propio banco. 



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


El paso más importante: nos olvidamos de actualizar WideNetFilter, agregamos una anulación para WideNetFilter::do_casewise(const FeeTxn&) const. En este ejemplo de "juguete", un error de este tipo puede parecer imperdonable de inmediato, pero en el código real, donde de un redefinidor a otro, docenas de líneas de código, y el lenguaje de una interfaz no virtual no se observa con mucho celo; creo que no será difícil encontrar uno class WideNetFilter : public NameWatchlistFilterque anule como do_genericeste y do_casewise, y piensa, “Oh, hay algo confuso aquí. Volveré a esto más tarde ”(y nunca volveré a esto).



En cualquier caso, espero que ya estés convencido de que a estas alturas tenemos un monstruo. ¿Cómo lo desencantamos ahora?



Refactorización, deshacerse de la herencia de implementación. Paso 1



Para deshacerse de la herencia de implementación enFilter::do_casewise, apliquemos el postulado de Scott Myers de que cualquier método virtual debe ser puro o (lógicamente) final. En este caso, esto es un compromiso, ya que aquí estamos rompiendo la regla de que las jerarquías deben ser superficiales. Introducimos una clase intermedia: 



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


Los filtros que proporcionan procesamiento sensible a mayúsculas y minúsculas para cada transacción posible ahora pueden simplemente heredar CasewiseFilter. Los filtros cuyas implementaciones son genéricas todavía heredan directamente de 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; }
};


Si alguien agrega un nuevo tipo de transacción y cambia CasewiseFilterpara incluir una nueva sobrecarga do_casewise, entonces veremos que nos hemos DifferentCustomerFilterconvertido en una clase abstracta: tenemos que lidiar con una transacción de un nuevo tipo. Ahora el compilador ayuda a cumplir con las reglas de nuestra arquitectura, donde solía ocultar silenciosamente nuestros errores.



También tenga en cuenta que ahora es imposible definir WideNetFilteren términos NameWatchlistFilter.



Refactorización, deshacerse de la herencia de implementación. Paso 2



Para deshacerse de la herencia de implementación en WideNetFilter, se aplica el principio de responsabilidad exclusiva . Por el momento, está NameWatchlistFilterresolviendo dos problemas: trabaja como un filtro en toda regla y tiene la habilidad is_flagged. Convirtámosla en is_flaggeduna clase independiente WatchlistGroupque no necesita ajustarse a la API class Filter, ya que no es un filtro, sino simplemente una clase auxiliar útil. 



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


Ahora se WideNetFilterpuede usar is_flaggedsin heredar NameWatchlistFilter. En ambos filtros, puede seguir la recomendación conocida y preferir la composición a la herencia, ya que la composición es una herramienta para reutilizar código, pero la herencia no lo es. 



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


Nuevamente, si alguien agrega un nuevo tipo de transacción y modifica CasewiseFilterpara incluir una nueva sobrecarga do_casewise, entonces nos aseguraremos de WideNetFilterconvertirnos en una clase abstracta: tenemos que lidiar con transacciones de un nuevo tipo en WideNetFilter(pero no en NameWatchlistFilter). ¡Es como si el compilador nos guardara una lista de tareas pendientes!



conclusiones



He anonimizado y simplificado enormemente este ejemplo en comparación con el código con el que tuve que trabajar. Pero el esquema general de la jerarquía de clases era exactamente eso, al igual que la lógica endeble do_generic(txn) && do_casewise(txn)del código original. Creo que por lo anterior no es tan fácil entender cuán imperceptiblemente se escondió el error en la estructura antigua FeeTxn. Ahora que le he presentado esta versión simplificada (¡literalmente la he masticado!), Yo mismo estoy prácticamente sorprendido, ¿era tan mala la versión original del código? Tal vez no ... después de todo, este código funcionó durante un tiempo.

Pero espero que esté de acuerdo en que la versión de refactorización que usa CasewiseFiltery especialmente WatchlistGroupresultó ser mucho mejor. Si tuviera que elegir con cuál de estas dos bases de código trabajar, no dudaría en tomar la segunda.


More articles:


All Articles