为了寻找灵感,以及如何补充出版社在C ++主题上的投资组合,我们遇到了Arthur O'Dwyer的博客,顺便说一句,他已经写了一本有关C ++的书,而这本书似乎无处不在。今天的帖子是关于干净代码的。我们希望案件本身和作者都会对您感兴趣。
我越是处理经典的多态代码,就越喜欢它的“现代”成语-非虚拟接口的成语,Liskov替换原理,Scott Myers的规则,即所有类必须是抽象的或最终的,任何层次结构必须严格地是两层的,并且基类表示接口统一性的规则,并且不要重用实现。
今天,我想向您展示一个代码片段,以演示“实现继承”如何导致问题以及我用来解开该问题的模式。不幸的是,令我烦恼的代码难以辨认,以至于我决定在此处以简化且略微针对特定领域的形式进行展示。我将尝试分阶段概述整个问题。
阶段1:交易
假设我们的主题领域是“银行交易”。我们有一个经典的多态接口层次结构。
class Txn { ... };
class DepositTxn : public Txn { ... };
class WithdrawalTxn : public Txn { ... };
class TransferTxn : public Txn { ... };
各种各样的事务都有某些通用的API,每种事务也都有自己的特定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;
};
阶段2:交易过滤器
但实际上,我们的程序不执行交易,而是跟踪交易以标记可疑交易。操作员可以设置满足某些条件的过滤器,例如“标记超过10,000美元的所有交易”或“标记代表清单W上的人员执行的所有交易”。在内部,我们将不同类型的操作员可配置过滤器表示为经典的多态层次结构。
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;
};
这是一个简单过滤器的示例:
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_;
};
阶段3:第一次跌倒
事实证明,某些过滤器实际上尝试访问特定于特定事务的API-上面已讨论了这些API。假设它
DifferentCustomerFilter
尝试标记执行者名称与发票中指定的名称不同的任何交易。为了举例说明,我们假设银行受到严格监管:只有该帐户的所有者才能从帐户中提取资金。因此,只有类DepositTxn
关注记录进行交易的客户的名称。
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;
}
}
};
这是对dynamic_cast的经典滥用!(经典-因为它总是被发现)。要修复此代码,可以尝试应用“ Classically polymorphic visit ”(2020-09-29)中的方法,但是不幸的是,未发现任何改进:
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;
});
}
};
因此,代码的作者想到了一个(讽刺!)想法。让我们在某些过滤器中实现区分大小写。让我们这样重写基类
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; }
};
这种巧妙的策略减少了您需要编写的代码量
DifferentCustomerFilter
。但是我们违反了OOP的原则之一,即禁止继承实现。该函数Filter::do_generic(const Txn&)
既不是纯函数也不是最终函数。这将再次困扰我们。
步骤4:第二次跌倒
现在让我们做一个过滤器,检查账户持有人是否在任何州黑名单上。我们要测试其中一些列表。
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_;
};
哦,我们需要制作另一个可以绘制更宽网格的过滤器-它将同时检查帐户持有人和用户名。同样,原始代码的作者有一个(讽刺!)聪明的主意。为什么要复制逻辑
is_flagged
,让我们更好地继承它。继承只是代码的重用,对吧?
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());
}
};
请注意,最终的体系结构是多么令人困惑。
NameWatchlistFilter
覆盖do_generic
以仅验证帐户持有人的姓氏,然后WideNetFilter
将其覆盖回到其原始视图。(如果我WideNetFilter
没有重新定义它,那么WideNetFilter
对于任何name_on_account()
未标记但已name_of_customer()
标记的存款交易,它都会错误地触发。)这很令人困惑,但目前仍然有效。
阶段5:一系列不愉快的事件
由于我们需要添加一种新型的交易,因此这种技术债务使我们陷入了一个意想不到的方向。让我们做一个类来代表银行本身发起的佣金和利息支付。
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; }
};
最重要的步骤:我们忘了更新
WideNetFilter
,为添加了替代WideNetFilter::do_casewise(const FeeTxn&) const
。这个错误的这种“玩具”的例子可能会立即显得不可原谅的,但在实际的代码,其中一个pereopredelitelya到另一几十行代码,和非虚拟接口的成语是不是太热心强制-我想,会不会是难以满足 class WideNetFilter : public NameWatchlistFilter
,重新定义了如何do_generic
使然后do_casewise
,想,“哦,这里有些混乱。稍后我将再次讨论”(永远不会再讨论这个问题)。
无论如何,我希望您已经说服了,到目前为止,我们已经有了一个怪物。我们现在如何分解他?
重构,摆脱实现继承。第1步
摆脱实现继承
Filter::do_casewise
,让我们应用斯科特·迈尔斯(Scott Myers)的假设,即任何虚拟方法都必须是纯方法或(逻辑上)最终方法。在这种情况下,这是一个折衷方案,因为在这里我们打破了层次结构应该浅的规则。我们介绍一个中间类:
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;
};
为每个可能的事务提供区分大小写处理的过滤器现在可以简单地从继承
CasewiseFilter
。实现通用的过滤器仍直接继承自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; }
};
如果有人添加了新的事务类型并进行了更改
CasewiseFilter
以包含新的重载do_casewise
,那么我们将看到我们已经DifferentCustomerFilter
成为抽象类:我们必须处理新类型的事务。现在,编译器有助于遵守我们的体系结构规则,该规则以前通常是用来隐藏我们的错误的。
另请注意,现在无法
WideNetFilter
按术语进行定义NameWatchlistFilter
。
重构,摆脱实现继承。第2步
为了摆脱实现继承
WideNetFilter
,适用唯一责任原则。目前,他正在NameWatchlistFilter
解决两个问题:它可以作为成熟的过滤器并具有的能力is_flagged
。让我们将其设为不需要遵循APIis_flagged
的独立类,因为它不是过滤器,而只是有用的帮助程序类。WatchlistGroup
class Filter
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_;
};
现在
WideNetFilter
可以使用is_flagged
而无需继承NameWatchlistFilter
。在这两个过滤器中,您可以遵循众所周知的建议,并且优选使用组合而不是继承,因为组合是一种可重用代码的工具,而继承则不是。
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_;
};
同样,如果有人添加了新的事务类型并进行修改
CasewiseFilter
以包括新的重载do_casewise
,那么我们将确保WideNetFilter
成为一个抽象类:我们必须在中WideNetFilter
(但不在中NameWatchlistFilter
)处理新类型的事务。就像编译器为我们保留了一个待办事项清单!
结论
与必须使用的代码相比,我已匿名且简化了此示例。但是类层次结构的总体轮廓正是这样,
do_generic(txn) && do_casewise(txn)
原始代码中的脆弱逻辑也是如此。我认为从上面的内容很难理解该错误在旧结构中的隐藏程度FeeTxn
。现在,我已经向您提供了此简化版本(字面意思是为您咀嚼了!),我自己对此感到非常惊讶,原始代码的版本是如此糟糕吗?也许不是……毕竟,这段代码工作了一段时间。
但我希望您会同意,使用
CasewiseFilter
和特别是重构版本的WatchlistGroup
结果要好得多。如果我必须选择使用这两个代码库中的哪一个,我会毫不犹豫地选择第二个。