When to setParent on injected QObjects?

shavera

This is maybe a more philosophical/architectural question, but suppose I have a class

class foo : public QObject{
public: 
    foo(dependency* child) : m_child(child), QObject() {}

    foo* makeFoo(){
        dependency* child = new dependency;
        return new foo(child);
    }
private:
    dependency* m_child;
}

Is it better to, in the constructor, set parent-child relationships (eg:

foo::foo(dependency* child) : m_child(child), QObject(){
    m_child->setParent(this);
}

or to set the relationship in the factory construction?

foo* foo::makeFoo(){
    dependency* child = new dependency;
    foo* _foo = new foo(child);
    child->setParent(_foo);
    return _foo;
}

I am pretty sure that they are functionally equivalent, but from a broader architectural stance, where should this relationship be set?

Chris Kawa

Just for the sake of consistency - it's highly unusual for a QObject derived class to take a child as a constructor parameter. They usually take a parent. For someone not familiar with your code, but familiar with Qt, something like new Foo(bar) would look like an instance of class Foo having a parent bar, which is the other way around in your code.

If you're planning to expose construction of class via make function then what's the point of passing a child as param at all? the object can create that child as part of its usual construction.

So my personal take on that design would be something like this:

class Foo : public QObject
{
public: 
   static Foo* makeFoo(QObject* parent = nullptr) { return new Foo(parent); }
protected:
    Foo(QObject* parent) : QObject(parent) {}
private:
    Dependency m_child;
}

Or, if you feel for whatever reason the child needs to be allocated dynamically (ask yourself really hard does it), something like this:

class Foo : public QObject
{
public: 
   static Foo* makeFoo(QObject* parent = nullptr) { return new Foo(parent); }
protected:
    //assuming Dependency takes parent as a constructor param
    Foo(QObject* parent) : QObject(parent), m_child(new Dependency(this)) {} 
private:
    Dependency* m_child;
}

alex_malyu

I would vote for explicit addChild(Object * ) if you really need to expose adding children functionality outside of the class

shavera

@Chris-Kawa
The reason I inject it is primarily for testing. Dependency injection allows me to inject mock objects at runtime. (Also, dependency injection as a general design principle for modularization, etc)

I can see the point in doing it in reverse, but in that case, I have to do explicit checks in the parent class as to whether the child class was injected or not. (eg:)

class Foo{ // with all attendent other stuff
public:
void DoStuff(){
  Dependency* child = findChild<Dependency*>();
  if(nullptr != child){
    child->doStuff;
  }
};

Which... I mean... works, but is really bloated to keep checking for children....

Oh: I just checked, there is a "childEvent" function in QObject, that I guess I could use to inject-at-runtime the class, but still seems like trying to bend my code to fit Qt, rather than Qt following some pretty standard coding paradigms.

Chris Kawa

I might be missing something but I'm not getting your point. In your original code you'd have to do the nullptr check just as well, because the child passed in the constructor could be nullptr.

shavera

@Chris-Kawa That's a valid point I hadn't considered. I'll just have to give this some more thought.

It seems like maybe the most "Qt-correct" design would be to override childEvent(QChildEvent* event) to take the pointer into the member object when a child is added to the parent object. This would then, at least, bypass the need to call "findChild" over and over again.