如何消除这种与继承相关的代码异味？
Posted
技术标签:
【中文标题】如何消除这种与继承相关的代码异味？【英文标题】：How do I remove this inheritance-related code smell? 【发布时间】：2019-10-30 19:39:02 【问题描述】：我需要实现很多具有不同 const 成员数据的派生类。数据处理应该在基类中处理，但我找不到访问派生数据的优雅方法。下面的代码可以运行，但我真的不喜欢它。
代码需要在小型嵌入式环境中运行，因此无法广泛使用堆或 Boost 等花哨的库。
class Base
public:
struct SomeInfo
const char *name;
const f32_t value;
;
void iterateInfo()
// I would love to just write
// for(const auto& info : c_myInfo) ...
u8_t len = 0;
const auto *returnedInfo = getDerivedInfo(len);
for (int i = 0; i < len; i++)
DPRINTF("Name: %s - Value: %f \n", returnedInfo[i].name, returnedInfo[i].value);
virtual const SomeInfo* getDerivedInfo(u8_t &length) = 0;
;
class DerivedA : public Base
public:
const SomeInfo c_myInfo[2] "NameA1", 1.1f, "NameA2", 1.2f ;
virtual const SomeInfo* getDerivedInfo(u8_t &length) override
// Duplicated code in every derived implementation....
length = sizeof(c_myInfo) / sizeof(c_myInfo[0]);
return c_myInfo;
;
class DerivedB : public Base
public:
const SomeInfo c_myInfo[3] "NameB1", 2.1f, "NameB2", 2.2f, "NameB2", 2.3f ;
virtual const SomeInfo *getDerivedInfo(u8_t &length) override
// Duplicated code in every derived implementation....
length = sizeof(c_myInfo) / sizeof(c_myInfo[0]);
return c_myInfo;
;
DerivedA instanceA;
DerivedB instanceB;
instanceA.iterateInfo();
instanceB.iterateInfo();
【问题讨论】：
不，我只实例化派生类。 @NikosC.Base 是抽象的，不能创建它的实例。 如果SomeInfo c_myInfo[3] 是const 并且有一个编译时常量初始化器，为什么你把它放在对象里面而不是static？您是否只为每种类型创建一个实例，所以实际上没有重复指针 + 浮点数？ （如果您将字符串键/值数组用作字典，它的效率听起来也不是很好，但这是一个单独的问题。听起来像是 enum.. 的工作）
感谢所有建议！到目前为止，Nikos C. answer 最适合我的需要，尽管 Peter Cordes 的方法也很简洁。只是一些澄清：1）一些用户建议将 c_myInfo 设为静态常量，他们当然是正确的。 2）我的嵌入式环境不是那么小，我必须计算每一位和字节。如果可以避免的话，我只是不想在库上编译一些额外的 10 kB。可读性比代码效率更重要。
【参考方案1】：
您在这里不需要任何虚拟或模板。只需将SomeInfo* 指针及其长度添加到Base，并提供一个受保护的构造函数来初始化它们（并且由于没有默认构造函数，因此不可能忘记初始化它们）。
受保护的构造函数不是硬性要求，但由于 Base 不再是抽象基类，因此使构造函数受保护会阻止 Base 被实例化。
class Base
public:
struct SomeInfo
const char *name;
const f32_t value;
;
void iterateInfo()
for (int i = 0; i < c_info_len; ++i)
DPRINTF("Name: %s - Value: %f \n", c_info[i].name,
c_info[i].value);
protected:
explicit Base(const SomeInfo* info, int len) noexcept
: c_info(info)
, c_info_len(len)
private:
const SomeInfo* c_info;
int c_info_len;
;
class DerivedA : public Base
public:
DerivedA() noexcept
: Base(c_myInfo, sizeof(c_myInfo) / sizeof(c_myInfo[0]))
private:
const SomeInfo c_myInfo[2] "NameA1", 1.1f, "NameA2", 1.2f ;
;
class DerivedB : public Base
public:
DerivedB() noexcept
: Base(c_myInfo, sizeof(c_myInfo) / sizeof(c_myInfo[0]))
private:
const SomeInfo c_myInfo[3]
"NameB1", 2.1f,
"NameB2", 2.2f,
"NameB2", 2.3f
;
;
您当然可以使用小型、零开销的包装器/适配器类来代替 c_info 和 c_info_len 成员，以提供更好、更安全的访问（如 begin() 和 end() 支持），但是这超出了这个答案的范围。
正如 Peter Cordes 指出的那样，这种方法的一个问题是，如果您的最终代码仍然使用虚拟对象（您没有使用的虚拟函数），那么派生对象现在会比指针大小加上 int显示在您的帖子中。）如果不再有虚拟对象，那么对象大小只会增加int。您确实说过您在一个小型嵌入式环境中，因此如果这些对象中有很多同时处于活动状态，那么这可能需要担心。
Peter 还指出，由于您的 c_myInfo 数组是 const 并且使用常量初始化器，因此您不妨将它们设为 static。这会将每个派生对象的大小减少数组的大小。
【讨论】：
这将每个对象的大小增加 1 个指针 + 1int。如果这些对象的实例多于派生类型，则 OP 的解决方案使用更少的总内存（数据 + 代码）。他们提到他们处于内存受限的嵌入式环境中。在高端系统上，额外的间接级别仍然是一个缺点。 （至少指针通常与数据的开头和 vtable 指针位于同一缓存行中，因此额外的延迟只是 1 L1d 缓存〜加载使用延迟。不是额外的缓存未命中）
@PeterCordes：请注意，OPs 解决方案将每个对象的大小增加 ???由于虚拟方法。这可能是 1 指针，但值得注意的是，大小回归比最初出现的要小。这也有更少间接，因为我们可以直接引用给定的指针，而OP的代码必须取消引用虚拟方法，然后调用它来获取指针，然后取消引用指针。
@MooingDuck：（成功）分支预测/推测隐藏了函数指针的延迟，但是如果您不需要多态性，这是一个非常糟糕的选择。如果多态性非常受内存限制，那么仅出于代码重用目的的多态性是值得的，但可能让静态知道类型的调用者将正确的参数传递给泛型函数是可行的方法，也许通过编写简单在每个调用点传递指针+大小的非虚拟内联辅助函数，或任何其他方式让 C++ 编译器发出执行此操作的机器代码。
@PeterCordes：“小型嵌入式环境”通常不具有诸如“分支预测/推测”之类的性能增强逻辑，因为它会显着增加晶体管数量（影响成本、尺寸和功耗）
@BenVoigt：我知道；只有高端微控制器才有分支预测；在低端，他们的短管道的分支延迟足够短，以至于他们只是在每个分支上吃掉它。在我最初的评论中，我谈到了在大核心 OoO CPU 的上下文中间接的性能成本，您关心数据依赖性和延迟（与吞吐量分开）。但是是的，在一个小的有序 CPU 上，如果你不需要它，虚拟调度会很糟糕。【参考方案2】：
您可以将 Base 设为模板并获取 const 数组的长度。像这样的：
template<std::size_t Length>
class Base
public:
struct SomeInfo
const char *name;
const float value;
;
const SomeInfo c_myInfo[Length];
void iterateInfo()
//I would love to just write
for(const auto& info : c_myInfo)
// work with info
;
然后从每个基类中相应地初始化数组：
class DerivedA : public Base<2>
public:
DerivedA() : Base<2> SomeInfo"NameA1", 1.1f, "NameA2", 1.2f
;
class DerivedB : public Base<3>
public:
DerivedB() : Base<3> SomeInfo"NameB1", 2.1f, "NameB2", 2.2f, "NameB2", 2.3f
;
然后照常使用。此方法消除了多态性并且不使用堆分配（例如，没有 std::vector），正如用户 SirNobbyNobbs 所要求的那样。
【讨论】：
为了节省代码大小，使iterateInfo成为一个非成员函数，它接受一个指针+长度作为函数参数，并为派生类提供简单的内联函数来执行iterateInfo(c_myInfo, Length);所以循环有一个共享实现和调用者只需传递参数。不幸的是，我不认为把它放在基类中会让大多数编译器只做一个定义，除非他们使用相同的代码折叠优化来合并相同的函数定义。【参考方案3】：
好的，那么让我们简化所有不必要的复杂性:)
您的代码实际上可以归结为以下内容：
SomeInfo.h
struct SomeInfo
const char *name;
const f32_t value;
;
void processData(const SomeInfo* c_myInfo, u8_t len);
SomeInfo.cpp
#include "SomeInfo.h"
void processData(const SomeInfo* c_myInfo, u8_t len)
for (u8_t i = 0; i < len; i++)
DPRINTF("Name: %s - Value: %f \n", c_myInfo[i].name, c_myInfo[i].value);
数据.h
#include "SomeInfo.h"
struct A
const SomeInfo info[2] "NameA1", 1.1f, "NameA2", 1.2f ;
static const u8_t len = 2;
;
struct B
const SomeInfo info[3] "NameB1", 2.1f, "NameB2", 2.2f, "NameB2", 2.3f ;
static const u8_t len = 3;
;
main.cpp
#include "data.h"
int
main()
A a;
B b;
processData(a.info, A::len);
processData(b.info, B::len);
【讨论】：
鉴于提供的示例，您是对的。但这只是为了突出我的问题而进行的简化。基类已经有几百行代码了，每个派生类也有更多的内容，而不仅仅是 const 信息。 好吧，我可以想象。我只能建议使用组合而不是继承以及简单的功能。编码可以是一种简单而愉快的体验。我们只是出于某种原因使一切复杂化:) @SirNobbyNobbs “基类已经有几百行代码了” - 这比您的简化示例可能突出的任何内容都更像是代码气味， 在每个结构中都有一个u8_t len = 3; 成员是没有意义的； info[] 数组成员的长度已经被静态地称为派生类型的一部分。 @SirNobbyNobbs：您可以做的是在 A 和 B 中的每一个中都有一个小的内联包装器，它将正确的参数传递给一个通用的 processData 函数。如果您需要，它可以 是虚拟的，但是当您拥有完整的类型信息时，让它内联到每个调用站点是好的。 （派生类型函数上的final 在更多情况下允许这样做。）
如果你想要求调用者手动内联，而不是写一个包装函数，然后让它static const int len;u8可能最终还是会花费你4字节，因为它在一个将获得 4 字节对齐的结构内部（假设alignof(float) = 4。所以结构大小将是 4 字节的倍数，因为编译器总是将结构填充到它们的alignof() 的倍数，所以在一个数组中所有元素都有正确的对齐方式。【参考方案4】：
您可以使用 CRTP：
template<class Derived>
class impl_getDerivedInfo
:public Base
virtual const SomeInfo *getDerivedInfo(u8_t &length) override
//Duplicated code in every derived implementation....
auto& self = static_cast<Derived&>(*this);
length = sizeof(self.c_myInfo) / sizeof(self.c_myInfo[0]);
return self.c_myInfo;
;
class DerivedA : public impl_getDerivedInfo<DerivedA>
public:
const SomeInfo c_myInfo[2] "NameA1", 1.1f, "NameA2", 1.2f ;
;
class DerivedB : public impl_getDerivedInfo<DerivedB>
public:
const SomeInfo c_myInfo[3] "NameB1", 2.1f, "NameB2", 2.2f, "NameB2", 2.3f ;
;
【讨论】：
getDerivedInfo 这里还需要虚拟吗？您也可以将重复的代码变成一个简单的调用者，用于一个需要指针 + 长度的基类泛型函数。这将为您提供简洁的语法，让编译器将指针+长度传递给常见的非重复实现。
@PeterCordes 我想getDerivedInfo 必须是虚拟的，因为这是问题中所做的假设（通过Base 类型的表达式访问对象）。泛型函数会很短，以至于无论如何都会被内联，从而产生相同的代码（初始化长度的表达式是一个常量表达式）。虚拟化可以手动实现，就像在 NikosC 中一样。以一种内存有效的方式回答，但编译器将不再能够执行去虚拟化。
@PeterCordes 该语言缺少一种表达基类中特定非静态数据成员值对于具有相同动态类型的所有对象相同的方式。【参考方案5】：
从词汇类型开始：
template<class T>
struct span
T* b = nullptr;
T* e = nullptr;
// these all do something reasonable:
span()=default;
span(span const&)=default;
span& operator=(span const&)=default;
// pair of pointers, or pointer and length:
span( T* s, T* f ):b(s), e(f)
span( T* s, size_t l ):span(s, s+l)
// construct from an array of known length:
template<size_t N>
span( T(&arr)[N] ):span(arr, N)
// Pointers are iterators:
T* begin() const return b;
T* end() const return e;
// extended container-like utility functions:
T* data() const return begin();
size_t size() const return end()-begin();
bool empty() const return size()==0;
T& front() const return *begin();
T& back() const return *(end()-1);
;
// This is just here for the other array ctor,
// a span of const int can be constructed from
// an array of non-const int.
template<class T>
struct span<T const>
T const* b = nullptr;
T const* e = nullptr;
span( T const* s, T const* f ):b(s), e(f)
span( T const* s, size_t l ):span(s, s+l)
template<size_t N>
span( T const(&arr)[N] ):span(arr, N)
template<size_t N>
span( T(&arr)[N] ):span(arr, N)
T const* begin() const return b;
T const* end() const return e;
size_t size() const return end()-begin();
bool empty() const return size()==0;
T const& front() const return *begin();
T const& back() const return *(end()-1);
;
此类型已通过 GSL 引入 C++ std（略有不同）。如果您还没有，上面的基本词汇类型就足够了。
跨度表示一个“指针”，指向已知长度的连续对象块。
现在我们可以谈谈span<char>：
class Base
public:
void iterateInfo()
for(const auto& info : c_mySpan)
DPRINTF("Name: %s - Value: %f \n", info.name, info.value);
private:
span<const char> c_mySpan;
Base( span<const char> s ):c_mySpan(s)
Base(Base const&)=delete; // probably unsafe
;
现在你的派生看起来像：
class DerivedA : public Base
public:
const SomeInfo c_myInfo[2] "NameA1", 1.1f, "NameA2", 1.2f ;
DerivedA() : Base(c_myInfo)
;
每个Base 有两个指针的开销。一个 vtable 使用一个指针，使您的类型抽象，添加间接性，并为每个 Derived 类型添加一个全局 vtable。
现在，理论上，您可以将其开销降低到数组的长度，并假设数组数据在Base 之后开始，但这是脆弱的、不可移植的，只有在绝望时才有用。
虽然您可能对嵌入式代码中的模板持谨慎态度（因为您应该具有任何类型的代码生成；代码生成意味着您可以从 O(1) 代码生成超过 O(1) 的二进制文件）。 span 词汇表类型很紧凑，如果您的编译器设置相当激进，则应该内联为空。
【讨论】：
【参考方案6】：CRTP + std::array 怎么样？没有额外的变量、v-ptr 或虚函数调用。 std::array 是 C 样式数组的一个非常薄的包装器。空基类优化确保不浪费空间。对我来说它看起来足够“优雅”:)
template<typename Derived>
class BaseT
public:
struct SomeInfo
const char *name;
const f32_t value;
;
void iterateInfo()
Derived* pDerived = static_cast<Derived*>(this);
for (const auto& i: pDerived->c_myInfo)
printf("Name: %s - Value: %f \n", i.name, i.value);
;
class DerivedA : public BaseT<DerivedA>
public:
const std::array<SomeInfo,2> c_myInfo "NameA1", 1.1f, "NameA2", 1.2f ;
;
class DerivedB : public BaseT<DerivedB>
public:
const std::array<SomeInfo, 3> c_myInfo "NameB1", 2.1f, "NameB2", 2.2f, "NameB2", 2.3f ;
;
【讨论】：
【参考方案7】：因此，如果您真的想按原样组织数据，我可以理解您为什么会在现实生活中这样做：
使用 C++17 的一种方法是返回一个表示您的内容列表的“视图”对象。然后可以在 C++11 for 语句中使用它。您可以编写一个将start+len 转换为视图的基本函数，因此您无需添加到虚方法cruft。
创建一个兼容 C++11 for 语句的视图对象并不难。或者，您可以考虑使用可以采用开始和结束迭代器的 C++98 for_each 模板：您的开始迭代器是 start；结束迭代器是start+len。
【讨论】：
【参考方案8】：您可以将数据移动到类之外的二维数组中，并让每个类返回一个包含相关数据的索引。
struct SomeInfo
const char *name;
const f32_t value;
;
const vector<vector<SomeInfo>> masterStore
"NameA1", 1.1f, "NameA2", 1.2f,
"NameB1", 2.1f, "NameB2", 2.2f, "NameB2", 2.3f
;
class Base
public:
void iterateInfo()
// I would love to just write
// for(const auto& info : c_myInfo) ...
u8_t len = 0;
auto index(getIndex());
for(const auto& data : masterStore[index])
DPRINTF("Name: %s - Value: %f \n", data.name, data.value);
virtual int getIndex() = 0;
;
class DerivedA : public Base
public:
int getIndex() override
return 0;
;
class DerivedB : public Base
public:
int getIndex() override
return 1;
;
DerivedA instanceA;
DerivedB instanceB;
instanceA.iterateInfo();
instanceB.iterateInfo();
【讨论】：
为什么const std::vector 用于已知固定大小的编译时间常数数据？似乎是一个平面 1D std::array<SomeInfo> 的工作，每个派生类都知道正确的起始索引 + 偏移量。 （GetIndex 返回std::pair<int,int>）。或std::array<std::vector<SomeInfo>>。或者，如果您只想按顺序迭代，则可以通过在每个子数组的末尾使用带有 nullptr 终止符的 SomeInfo 对象的平面数组来隐式长度。
OP 没有共享完整的代码，因此无论数据是否为编译时间常数，这都是一种抛硬币的方式。
好的，但是使用vector<vector<T>> 超过array<vector<T>> 肯定是零点。外部数组绝对是固定大小的，因为每个派生类型都有 1 个条目。【参考方案9】：
只需让虚函数直接返回对数据的引用（然后您需要更改为向量 - 对于不同大小的数组或 C 样式数组类型是不可能的）：
virtual const std::vector<SomeInfo>& getDerivedInfo() = 0;
或者如果指针是唯一可行的选项，作为指针范围（如果可能的话，迭代器/范围适配器将是首选 - 更多）：
virtual std::pair<SomeInfo*, SomeInfo*> getDerivedInfo() = 0;
要使最后一种方法与 基于范围的 for 循环一起使用：一种方法是创建一个具有 begin()/end() 功能的小型“范围视图”类型 - 必须与 begin()/end() 配对
示例：
template<class T>
struct ptr_range
std::pair<T*, T*> range_;
auto begin()return range_.first;
auto end()return range_.second;
;
然后构造它：
virtual ptr_range<SomeInfo> getDerivedInfo() override
return std::begin(c_myInfo), std::end(c_myInfo);
如果不需要模板，很容易将其设为非模板。
【讨论】：
以上是关于如何消除这种与继承相关的代码异味？的主要内容，如果未能解决你的问题，请参考以下文章
SonarQube 代码有异味，因为它在三个选项中的两个选项中忽略了“this”
如何修复 Next.js 中 _app.js 中包含的 Sonarqube“重命名此文件”代码异味？