Clever Geek Handbook

Herança de implementação em C ++. História real

Olá, Habr!



Em busca de inspiração para adicionar ao portfólio da editora sobre C ++, encontramos o blog de Arthur O'Dwyer, que, aliás, já havia escrito um livro sobre C ++, que surgiu do nada . A postagem de hoje é sobre código limpo . Esperamos que tanto o caso em si quanto o autor sejam de seu interesse.



Quanto mais lido com o código polimórfico clássico, mais aprecio os idiomas "modernos" que superaram - o idioma de uma interface não virtual, o princípio de substituição de Liskov , a regra de Scott Myers de que todas as classes devem ser abstratas ou finais, a regra de que qualquer a hierarquia deve ser estritamente de dois níveis e a regra de que as classes básicas expressam a unidade da interface e não reutilizam a implementação.



Hoje, quero mostrar a você um trecho de código que demonstra como a "herança de implementação" levou a problemas e quais padrões usei para desvendá-la. Infelizmente, o código que me perturbou era tão ilegível que aqui decidi mostrá-lo de uma forma simplificada e ligeiramente específica para o domínio. Tentarei delinear todo o problema em etapas.



Etapa 1: Transações



Digamos que nossa área de assunto seja "transações bancárias". Temos uma hierarquia de interface polimórfica clássica. 



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


Uma grande variedade de transações tem certas APIs comuns e cada tipo de transação também tem sua própria 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;
};


Estágio 2: Filtros de transação



Mas, na realidade, nosso programa não executa transações, mas as rastreia para sinalizar transações suspeitas. O operador humano pode definir filtros que atendam a certos critérios, como “sinalizar todas as transações acima de $ 10.000” ou “sinalizar todas as transações realizadas em nome de pessoas na lista de verificação W”. Internamente, representamos os diferentes tipos de filtros configuráveis ​​pelo operador como uma hierarquia polimórfica clássica. 



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


Aqui está um exemplo de filtro simples: 



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: tropeçou na primeira vez



Acontece que alguns filtros realmente tentam acessar as APIs que são específicas para transações específicas - essas APIs foram discutidas acima. Digamos que ele esteja DifferentCustomerFiltertentando marcar qualquer transação em que o nome de seu executor seja diferente do nome especificado na fatura. Por exemplo, vamos supor que o banco seja estritamente regulamentado: somente o dono dessa conta pode sacar dinheiro de uma conta. Portanto, apenas a classe se DepositTxnpreocupa em registrar o nome do cliente que realizou a transação. 



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 é um abuso clássico de dynamic_cast! (Clássico - porque é encontrado o tempo todo). Para corrigir este código, pode-se tentar aplicar o método de “ Visita polimórfica clássica ” (2020-09-29), mas, infelizmente, nenhuma melhoria foi observada: 



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


Portanto, o autor do código foi atingido (sarcasmo!) Por uma ideia. Vamos implementar a diferenciação de maiúsculas e minúsculas em alguns filtros. Vamos reescrever a classe base Filterassim: 



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


Essa tática inteligente reduz a quantidade de código que você precisa escrever DifferentCustomerFilter. Mas estamos violando um dos princípios da OOP, a saber, a proibição de herança de implementação. A função Filter::do_generic(const Txn&)não é pura nem final. Isso vai voltar para nos assombrar.



Passo 4: Tropeçou uma segunda vez



Agora vamos fazer um filtro que verifica se o titular da conta está na lista negra de algum estado. Queremos testar várias dessas 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, precisamos fazer outro filtro que desenhe uma grade mais ampla - ele verificará o titular da conta e o nome de usuário. Novamente, o autor do código original tem uma (sarcasmo!) Idéia inteligente. Por que duplicar a lógica is_flagged, vamos herdá-la melhor. Herança é apenas uma reutilização de código, certo? 



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 como a arquitetura resultante é terrivelmente confusa. NameWatchlistFiltersubstitui do_genericpara validar apenas o sobrenome do titular da conta e, em seguida, o WideNetFiltersubstitui de volta à visualização original. (Se eu WideNetFilternão tivesse redefinido de volta, WideNetFilterteria funcionado incorretamente para qualquer transação de depósito onde não name_on_account()estivesse marcado, mas name_of_customer()marcado.) Isso é confuso, mas funciona ... por enquanto.



Etapa 5: uma série de eventos desagradáveis



Essa dívida técnica nos levou a uma direção tão inesperada, pois precisávamos adicionar um novo tipo de transação. Vamos fazer uma classe para representar comissões e juros iniciados pelo próprio 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; }
};


A etapa mais importante: esquecemos de atualizar WideNetFilter, adicionar uma substituição para WideNetFilter::do_casewise(const FeeTxn&) const. Neste exemplo de "brinquedo", tal erro pode parecer imediatamente imperdoável, mas em código real, onde de um redefinidor para outro dezenas de linhas de código e o idioma de uma interface não virtual não é observado com muito zelo - acho que não será difícil encontrar um class WideNetFilter : public NameWatchlistFilterque se sobreponha como do_genericeste e do_casewise, e pensar: “Oh, algo está confuso aqui. Voltarei a isso mais tarde ”(e nunca mais voltarei a isso).



De qualquer forma, espero que você já esteja convencido de que, a esta altura, temos um monstro. Como podemos desencantá-lo agora?



Refatoração, livrando-se da herança de implementação. Etapa 1



Para se livrar da herança de implementação emFilter::do_casewise, vamos aplicar o postulado de Scott Myers de que qualquer método virtual deve ser puro ou (logicamente) final. Nesse caso, isso é um meio-termo, já que aqui estamos quebrando a regra de que as hierarquias devem ser superficiais. Apresentamos uma classe intermediária: 



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


Filtros que fornecem processamento com distinção entre maiúsculas e minúsculas para todas as transações possíveis agora podem simplesmente herdar de CasewiseFilter. Filtros cujas implementações são genéricas ainda herdam diretamente 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; }
};


Se alguém adicionar um novo tipo de transação e mudar CasewiseFilterpara incluir uma nova sobrecarga do_casewise, veremos que nos DifferentCustomerFiltertornamos uma classe abstrata: temos que lidar com uma transação de um novo tipo. Agora o compilador ajuda a cumprir as regras de nossa arquitetura, onde costumava esconder nossos erros silenciosamente.



Observe também que agora é impossível definir WideNetFilterem termos NameWatchlistFilter.



Refatoração, livrando-se da herança de implementação. Passo 2



Para se livrar da herança de implementação no WideNetFilter, o princípio de responsabilidade exclusiva se aplica . No momento, ele está NameWatchlistFilterresolvendo dois problemas: funciona como um filtro completo e tem a capacidade is_flagged. Vamos torná-la is_flaggeduma classe autônoma WatchlistGroupque não precisa estar em conformidade com a API class Filter, já que não é um filtro, mas apenas uma classe 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_;
};


Agora WideNetFilterpode usar is_flaggedsem herdar NameWatchlistFilter. Em ambos os filtros, você pode seguir a recomendação conhecida e preferir a composição à herança, já que a composição é uma ferramenta para reutilizar o código, mas a herança não é. 



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


Novamente, se alguém adiciona um novo tipo de transação e modifica CasewiseFilterpara incluir uma nova sobrecarga do_casewise, então faremos questão de nos WideNetFiltertornar uma classe abstrata: temos que lidar com transações de um novo tipo em WideNetFilter(mas não em NameWatchlistFilter). É como se o compilador mantivesse uma lista de tarefas para nós!



conclusões



Eu tornei anônimo e simplifiquei extremamente este exemplo em comparação com o código que tive que trabalhar. Mas o esboço geral da hierarquia de classes era exatamente isso, assim como a lógica frágil do_generic(txn) && do_casewise(txn)do código original. Acho que pelo exposto não é tão fácil entender o quão imperceptivelmente o bug estava escondido na estrutura antiga FeeTxn. Agora que apresentei esta versão simplificada para você (literalmente mastiguei para você!), Eu mesmo estou praticamente surpreso comigo mesmo, a versão original do código era tão ruim? Talvez não ... afinal, esse código funcionou por um tempo.

Mas espero que você concorde que a versão de refatoração usando CasewiseFiltere, especialmente, WatchlistGroupacabou sendo muito melhor. Se eu tivesse que escolher com qual dessas duas bases de código trabalhar, não hesitaria em escolher a segunda.


More articles:


All Articles