Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How do I remove this inheritance-related code smell?

I need to implement a lot of derived classes with different const member data. The data processing should be handled in the base class, but I can't find an elegant way to access the derived data. The code below is working, but I really don't like it.

The code needs to run in a small embedded environment so extensive usage of the heap or fancy libraries like Boost is no option.

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(); 
like image 557
SirNobbyNobbs Avatar asked Jun 17 '19 08:06

SirNobbyNobbs


2 Answers

You don't need any virtuals or templates here. Just add a SomeInfo* pointer and its length to Base, and provide a protected constructor to initialize them (and since there's no default constructor, it won't be possible to forget to initialize them).

The constructor being protected is not a hard requirement, but since Base is not an abstract base class anymore, making the constructor protected prevents Base from being instantiated.

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

You can of course use a small, zero-overhead wrapper/adapter class instead of the c_info and c_info_len members in order to provide nicer and safer access (like begin() and end() support), but that's outside the scope of this answer.

As Peter Cordes pointed out, one issue with this approach is that the derived objects are now larger by the size of a pointer plus the size of an int if your final code still uses virtuals (virtual functions you haven't showed in your post.) If there's no virtuals anymore, then object size is only going to increase by an int. You did say that you're on a small embedded environment, so if a lot of these objects are going to be alive at the same time, then this might be something to worry about.

Peter also pointed out that since your c_myInfo arrays are const and use constant initializers, you might as well make them static. This will reduce the size of each derived object by the size of the array.

like image 182
Nikos C. Avatar answered Sep 29 '22 15:09

Nikos C.


You could make Base a template and take the length of your const array. Something like this:

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

And then initialize the array accordingly from each base class:

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

And then use as you normally would. This method removes the polymorphism and uses no heap allocation (e.g. no std::vector), just as user SirNobbyNobbs requested.

like image 22
DeiDei Avatar answered Sep 29 '22 14:09

DeiDei