0

I am trying to reduce code duplication in an init function that initializes static members of derived classes. In my case, this function is called initTypeInfo. Originally, each of my child classes ChildA, ChildB ... ChildN had their own initTypeInfo. Once I decided to move initTypeInfo into the Parent class because pretty much all the code except the namespace of the static member (ChildA::s_typeInfo vs. ChildB::s_typeInfo ...) was the same, I started to see problems with the static member s_typeInfo. This variable is a smart pointer of type std::shared_ptr<TypeInfo>

Here is my code sample:

#include <string>
#include <memory>
#include <iostream>

struct TypeInfo
{
    std::string name;
    // other members ...
};

class Parent 
{
    public:
    Parent(std::string name)
    : m_name(name)
    {

    }

    virtual std::shared_ptr<TypeInfo> getTypeInfo() = 0;

    virtual void initTypeInfo()
    {
        // this function contains lots of common code, trying to avoid code duplication 

        std::shared_ptr<TypeInfo> typeInfo = getTypeInfo();

        // initialize if it hasn't already been initialized 
        if (typeInfo == nullptr)
        {
            TypeInfo info {m_name};
            // the variable Child<n>::s_typeInfo doesn't point to this allocated data :(
            typeInfo = std::make_shared<TypeInfo>(info);
        }
    }

    private:
    std::string m_name;
};

class ChildA : public Parent
{
    public:
    ChildA(std::string name  /*other params...*/)
    : Parent(name)
    {
        initTypeInfo();
    }

    std::shared_ptr<TypeInfo> getTypeInfo()
    {
        return s_typeInfo;
    }

    private:    
    inline static std::shared_ptr<TypeInfo> s_typeInfo = nullptr;
    // other members...
};


class ChildB : public Parent
{
    public:
    ChildB(std::string name  /*other params...*/)
    : Parent(name)
    {
        initTypeInfo();
    }

    std::shared_ptr<TypeInfo>  getTypeInfo()
    {
        return s_typeInfo;
    }

    private:    
    inline static std::shared_ptr<TypeInfo> s_typeInfo = nullptr;
    // other members...
};


int main()
{
    ChildA ca1("childa1");
    ChildA ca2("childa2");

    ChildB cb1("Childb1");

    return 1;
}

So my questions are:

  1. Is this bad design to begin with, where I initialize the static member in the parent class
  2. How can the code duplication here be addressed?
  3. I know returning by value is the preferred way to return smart pointers from a function. But in this case, how can I have my member Child<n>::s_typeInfo point to the data I allocated in Parent::initTypeInfo()

The links I followed online pointed me to this link so I suppose I could achieve this with templates, but I'm not convinced that's the only way to do this.

Edit1:

In practice TypeInfo doesn't only contain one data member. It's a complex class with several data members and methods. Initially I had initTypeInfo functions in each derived class which would initialize each s_typeInfo. However, turns out that the logic in all these initTypeInfo functions were the same! The only difference was that the specific s_typeInfo of each derived class had to be initialized. So I wanted a way to transfer the logic into the Parent class (thereby reducing duplication) but also init the static s_typeNode in the parent. I guess, alternatively the question could be, how do I get the child class shared_ptr to point to the TypeInfo I create in Parent::initTypeInfo()?

The reason s_typeInfo exists as a static member is because I want to share that class across all instances of a certain Child. So in my main if for example I start 4 instance of ChildA I need them to share a single ChildA::s_typeInfo and if I have 10 instances of ChildB, I need them to share a single ChildB::s_typeInfo.

Edit2:

Adding minimal example of code for what I want to achieve. I was expecting *(c.p) to be 1 instead of nullptr as I had returned from C::get() by reference.

class C
{
    public:
    std::shared_ptr<int> & get() 
    {
        return p;
    }
    inline static std::shared_ptr<int> p = nullptr;
};

int main()
{
    C c;

    std::shared_ptr<int> p1 = c.get();

    if (p1 == nullptr)
    {
        p1 = std::make_shared<int>(1);
    }
    
    std::cout << *p1 << std::endl;

    if (c.p == nullptr)
    {
        std::cout << "nullptr" << std::endl;
    }
    else 
    {
        std::cout << *(c.p) << std::endl;
    }

    return 0;
}
simplename
  • 717
  • 7
  • 15
  • Why not initialize it in constructor then? – Tony Tannous Jul 27 '21 at 23:53
  • 2
    The base class should handle base class initialization and derived classes should handle their own initialization. – Retired Ninja Jul 27 '21 at 23:53
  • 1
    You might want to look up CRTP pattern – Ranoiaetep Jul 27 '21 at 23:55
  • The shown logic seems to be flawed. I am unable to find anything that actually initializes `childA::s_typeInfo` and `childB::s_typeInfo`. There are functions that return their values. But absolutely nothing that actually initializes them. – Sam Varshavchik Jul 28 '21 at 00:01
  • 1
    It is quite horrendous. Initializing static members from constructors in **never** a great idea, and usually ends up in flames. From the snippet it is not clear why any of this is needed. What stops you from simply having a static data member in each class (TypeInfo, and not even a shared pointer!) which is statically initialized as required. Than you can have virtual getTypeInfo() returning a reference to this static member in each class supporting typeInfo. It would be a duplication, but a duplication of a single line. – SergeyA Jul 28 '21 at 01:22
  • *"I started to see the following problems"* -- no description of the problems are in the text following this statement. If you are going to tell us about the problems (which is a Good Idea), please do tell us about the problems. – JaMiT Jul 28 '21 at 01:56
  • In addition to describing the issues you faced, you might want to take a step back and present your goals and requirements. You seem to have an inkling that you might be presenting an [XY problem](https://en.wikipedia.org/wiki/XY_problem), so go ahead and open the door to answers that address your original problem. Why (on an abstract level) do your classes need `TypeInfo`? Why does each class need its own `TypeInfo`? Stating *"pretty much all the code [...] was the same"* implies that some of the code differed -- what are those differences, and why are they needed? – JaMiT Jul 28 '21 at 02:07
  • @Retired Ninja that's how I had it at first, but that meant the `initTypeInfo` function was present in all derived classes, and the code in there was duplicated. – simplename Jul 28 '21 at 05:53
  • @Sam Varshavchik the initialization of all the `ChildN::s_typeInfo` used to be in `initTypeInfo` functions in each derived class. But the logic there was getting duplicated as it was the same for each derived class. Now the idea is that the `Parent::initTypeInfo()` will initialize `s_typeInfo` – simplename Jul 28 '21 at 05:57
  • Unfortunately, `Parent::initTypeInto()` does absolutely nothing of that sort. Can you point your finger at which line of that function that does that? That `if` statement's code does not, I repeat, initialize any child class's `s_typeinfo`. You may think it does because a ***copy*** of it ends up in it's local variable `typeinfo`, and it gets overwritten. The copy gets overwritten, not the original. C++ does not work this way. – Sam Varshavchik Jul 28 '21 at 11:40
  • @Sam Varshavchik I know it doesn't! That's my question #3 how can I initialize `s_typeInfo` from `Parent::initTypeInfo`? – simplename Jul 28 '21 at 12:41
  • Return it by reference, instead of value. Are you familiar with references, in C++? – Sam Varshavchik Jul 28 '21 at 12:45
  • @SamVarshavchik As far as I understand, I should return smart pointers by val;ue, and shouldn't that just increment the count and give me an added reference? Either way I tried returning by reference and this problem still exists -- `ClassN::s_typeInfo` is `nullptr` initially, then in `Parent::initTypeInfo` I call `ChildN::getTypeInfo` (which I tried returning by value or reference) as seen in my sample code above. Then I initialize that returned pointer by after I do that `ClassN::s_typeInfo` is still `nullptr` – simplename Jul 29 '21 at 03:28
  • I don't know what "tried returning by reference" means. I only understand actual code, and not paraphrased descriptions of it. It's true that returning by value increments the reference count of what `s_typeInfo` would point to. Except that, initially, it's not pointing to anything. It's NULL. And even if it wasn't, incrementing the reference count accomplishes absolutely nothing whatsoever, here, since `s_typeInfo` itself doesn't change. There's nothing wrong with returning anything by reference, as long as you fully understand what it means. See your C++ textbook for more info on references. – Sam Varshavchik Jul 29 '21 at 10:56
  • @SamVarshavchik I added a sample code to my question of me returning a `shared_ptr` by reference, then initializing it. But the `C::p` in my example still is `nullptr` after that. – simplename Jul 29 '21 at 13:58
  • 1
    You're returning it by reference but then the returned reference is stored in a non-reference variable. This is no different than returning it by value. This needs to be returned by reference ***and*** stored in a reference variable. See your C++ textbook for more information on references. – Sam Varshavchik Jul 29 '21 at 21:19

0 Answers0