Cyclic dependencies, observer, or...

Hi, I'm having a problem to design my project. I'm using open scene graph to render the 3D representation. I have an osg:Group (eg. MyGroup) object that will become a leaf node / child of the root node. Inside the osg::Group object, I've got multiple osg::Node objects (eg. MyNodeA, MyNodeB, etc) as the leaf nodes / children. Now, one of the osg::Node objects depends on another osg::Node's attribute for rendering. I need some of the attributes of the other osg::Node to be sent as a uniform value of the shader.

My question is, how do I design this? I've got a few ideas. The first one that came out of my mind is by doing cyclic dependencies like this pseudo code:

Code:

---

MyGroup.h

#include "MyNodeA.h"
#include "MyNodeB.h"
#include ...

class MyGroup:: public osg::Group
{
  ...
  public:
    //Mutators & accessors
    void setNodeA(const MyNodeA* node);
    MyNodeA* getNodeA( ) const;

    void setNodeB(const MyNodeB* node);
    MyNodeB* getNodeB( ) const;

    void setNodeB(const MyNodeC* node);
    MyNodeB* getNodeC( ) const;
 
  private:
    MyNodeA* mNodeA;
    MyNodeB* mNodeB;
    MyNodeC* mNodeC;
}

----------------------
MyGroup.cpp

#include "MyGroup.h"
#include ...

...


----------------------
MyNodeA.h

#include "MyNodeB.h"
#include "MyNodeC.h"

class MyGroup; //< Forward decl

class MyNodeA:: public osg::Node
{
  public:

    //Mutators & accessors
    void setGroup(const MyGroup* group);
    MyGroup* getGroup( ) const;

    void checkNodeB();
    void updateUniformVals(const MyNodeB* nodeB);

    void checkNodeC();
    void updateUniformVals(const MyNodeC* nodeC);
 
  private:
    MyGroup* mGroup;
 
}

----------------------
MyNodeA.cpp

#include "MyGroup.h"
#include "MyNodeA.h"
#include ...

...

void MyNodeA::checkNodeB()
{
  updateUniformVals(getGroup()->getNodeB());
}

void MyNodeA::updateUniformVals(const MyNodeB* nodeB)
{
 ....
}

void MyNodeA::checkNodeC()
{
  updateUniformVals(getGroup()->getNodeC());
}

void MyNodeA::updateUniformVals(const MyNodeC* nodeC)
{
 ....
}

---

The second one is by using the observer pattern:

Code:

---

Observer.h

class Subject;

class iObserver
{
  public:
    virtual void update(Subject* subject) = 0;
}

----------------------
Subject.h
#include <vector>
#include "Observer.h"

class Subject
{
  private:
    vector<iObserver*> vecObservers;
  public:
    attach(iObserver& obs);
    detach(iObserver& obs);
    void notify(); //calls vecObservers[i]->update(this);
}

----------------------
MyGroup.h

#include "MyNodeA.h"
#include "MyNodeB.h"
#include "MyNodeC.h"
#include ...

class MyGroup: public osg::Group
{
  ...
  public:
    MyGroup(MyNodeA* nodeA, MyNodeB* nodeA, MyNodeC* nodeA)
    {
        mObserverAB = new RenderStateObserverAB(nodeA, nodeB);
        mObserverAC = new RenderStateObserverAB(nodeA, nodeC);

        nodeA->attach(mObserverAB);
        nodeA->attach(mObserverAC);
        nodeB->attach(mObserverAB);
        nodeC->attach(mObserverAC);
    }   
 
  private:
    RenderStateObserverAB *mObserverAB; //< observer between AB
    RenderStateObserverAC *mObserverAC; //< observer between AC

}

----------------------
MyGroup.cpp

#include "MyGroup.h"
#include ...

....

-----------------
RenderStateObserverAB.h

#include "Observer.h"
#include "MyNodeA.h"
#include "MyNodeB.h"

class RenderStateObserverAB: public iObserver
{
  ...
  public:
    RenderStateObserverAB(MyNodeA* nodeA, MyNodeB* nodeA)
    {
        mNodeA = nodeA;
        mNodeB = nodeB;
    }

    virtual void update(Subject* subject)
    {
        MyNodeA->updateUniformVals(mNodeB);
    }
 
  private:
    MyNodeA *mNodeA;
    MyNodeB *mNodeB;

}

-----------------
RenderStateObserverAB.h

#include "Observer.h"
#include "MyNodeA.h"
#include "MyNodeB.h"

class RenderStateObserverAB: public iObserver
{
  ...
  public:
    RenderStateObserverAC(MyNodeA* nodeA, MyNodeC* nodeC)
    {
        mNodeA = nodeA;
        mNodeC = nodeC;
    }

    virtual void update(Subject* subject)
    {
        MyNodeA->updateUniformVals(mNodeC);
    }
 
  private:
    MyNodeA *mNodeA;
    MyNodeC *mNodeC;

}

----------------------
MyNodeA.h

#include "Subject.h"
#include "MyNodeB.h"
#include "MyNodeC.h"

class MyGroup; //< Forward decl

class MyNodeA:: public osg::Node, Subject
{
  ...
  public:

    void updateUniformVals(const MyNodeB* nodeB);
    void updateUniformVals(const MyNodeC* nodeC);
 
  private:
  ...
 
}

----------------------
MyNodeA.cpp

#include "MyNodeA.h"
#include ...

...

void MyNodeA::updateUniformVals(const MyNodeB* nodeB)
{
 ....
}

void MyNodeA::updateUniformVals(const MyNodeC* nodeC)
{
 ....
}

---

I know that those two approach is not entirely good. Heck, I don't even know if I've used the Observer pattern right or not here. Or even is it the right thing to do to use the pattern. So can you guys give me some suggestions if you happen to know how to design it better. Thanks.

Code:

---

Of the two, I'd go for the first one - it's much simpler, which in my book is a damn good thing to aim for. In my experience, design patterns are simply hacks around language deficiencies, and the more "general purpose" they are, the more complicated they have to be, generally detracting from the point of code, which is to get the message across.

Although I do have a specific worry:

class MyGroup:: public osg::Group
{
    void setNodeB(const MyNodeB* node);
    MyNodeB* getNodeB( ) const;

    void setNodeB(const MyNodeC* node);
    MyNodeB* getNodeC( ) const;
}

void MyNodeA::checkNodeB()
{
  updateUniformVals(getGroup()->getNodeB());
}

void MyNodeA::updateUniformVals(const MyNodeB* nodeB)
{
 ....
}

void MyNodeA::checkNodeC()
{
  updateUniformVals(getGroup()->getNodeC());
}

void MyNodeA::updateUniformVals(const MyNodeC* nodeC)
{
 ....
}

Although if MyNode{A,B,C} are specific subclasses, this isn't extensible to future subclasses - you'll have to add a pair of methods to MyGroup every time you subclass is.

It sounds from your vague description that what you might want is some setRenderProperties() method to set the things other nodes depend upon for consistent rendering - then one chosen "primary" node registers its properties, and the other's get it through some getRenderProperties() method. Unless I'm missing something...

---

Note: Above post is entirely within code tags, as when I put just the code in code tags, the forum rejected my post and told me to "please put code within code tags - read forum rules etc.". I hate it when I can't override things like this - can't it just trust me? Bah!

Thanks for the reply. I thought we should always use Design Pattern at all time because it's the right thing to do? FYI, I'm a sort of cowboy coder who coded from what I see fit at that instant, even if the end result is very complicated with lots of coupling or, in a way, screwed. So I wanted to try changing my way of coding by designing them all first before doing anything else. Okay, I'll elaborate more on the situation. What I'm trying to do is making an atmosphere rendering group (AtmosphereGroup class) based on Preetham's scattering model. It consists of 3 nodes: the sky dome (SkyDomeNode class), spherical sun (SunNode class), and volumetric clouds (CloudsNode class). The sky dome object is dependent on the sun to calculate the scattering, the clouds object is dependent on the sky dome's sky color. In the atmosphere group class, I probably need to make an object to handle the simulation time and then convert it to the sun's position and direction. I think what I probably need is how to design this so that every node can aware to each other, so if one class changed, the rest will follow. But then again, I just need to only know whether the sun node changed its state or not, and the sky dome and clouds will follow.

Quote:

---

Originally Posted by JohnGraham

Note: Above post is entirely within code tags, as when I put just the code in code tags, the forum rejected my post and told me to "please put code within code tags - read forum rules etc.". I hate it when I can't override things like this - can't it just trust me? Bah!

---

Put all code within code tags, and avoid putting { and } outside code tags.

LoL. And I thought you're replying to my question. :P

Anyone can give me some insight here?

Quote:

---

I thought we should always use Design Pattern at all time because it's the right thing to do?

---

That's a matter of opinion - some would agree, some would not. I'm somewhere in between - they can be good to study to see various methods of doing things, but should be avoided if they are too complicated for your particular situation. I like the philosophy (can't remember who said it - Paul Graham I think?) of do the simplest thing that works, and your design pattern example above is much less simple than your other approach.

Quote:

---

What I'm trying to do is making an atmosphere rendering group (AtmosphereGroup class) based on Preetham's scattering model. It consists of 3 nodes: the sky dome (SkyDomeNode class), spherical sun (SunNode class), and volumetric clouds (CloudsNode class). The sky dome object is dependent on the sun to calculate the scattering, the clouds object is dependent on the sky dome's sky color. In the atmosphere group class, I probably need to make an object to handle the simulation time and then convert it to the sun's position and direction. I think what I probably need is how to design this so that every node can aware to each other, so if one class changed, the rest will follow. But then again, I just need to only know whether the sun node changed its state or not, and the sky dome and clouds will follow.

---

Focusing on SunNode and SkyDome, it sounds like you simply need to tell one about the other. You could have a SunNode pointer in your SkyDome class (or an array if you want to have more than one sun) so that it knows where to get its rendering information from:

Code:

---

class SunNode {};

class SkyDome
{
private:
    SunNode *sun;

public:
    void setSun(SunNode *newSun)
    {
        this->sun = newSun;
    }

    void render()
    {
        if (!this->sun) {
            fprintf(stderr, "Warning: Asked to render with no sun");
            return;
        }

        // Rendering...
    }
};

---

If you only want to update the SkyDome when the SunNode is updated, give SunNode control over the SkyDome so it can prompt it at the appropriate time.

Sorry it took a while for me to respond.

Quote:

---

Originally Posted by JohnGraham

That's a matter of opinion - some would agree, some would not. I'm somewhere in between - they can be good to study to see various methods of doing things, but should be avoided if they are too complicated for your particular situation. I like the philosophy (can't remember who said it - Paul Graham I think?) of do the simplest thing that works, and your design pattern example above is much less simple than your other approach.




Focusing on SunNode and SkyDome, it sounds like you simply need to tell one about the other. You could have a SunNode pointer in your SkyDome class (or an array if you want to have more than one sun) so that it knows where to get its rendering information from:

Code:

---

class SunNode {};

class SkyDome
{
private:
    SunNode *sun;

public:
    void setSun(SunNode *newSun)
    {
        this->sun = newSun;
    }

    void render()
    {
        if (!this->sun) {
            fprintf(stderr, "Warning: Asked to render with no sun");
            return;
        }

        // Rendering...
    }
};

---

If you only want to update the SkyDome when the SunNode is updated, give SunNode control over the SkyDome so it can prompt it at the appropriate time.

---

Ok, again, that's actually the very first idea, and the one I usually use when I see this kind of problem. Currently, if I need to know a certain parameter of an object, I will just make an instance of it as a member variable and just make a mutator/accessor method for it. So, eg. if I need object A, I'll just add the instance of A into the class that need them, object B, and I'll just add an instance of object B, etc... etc... and so on... And when the project got bigger, and I looked at the code again, all I see is a mess of tangled instances. Too many dependencies to other classes and too many coupling. Is it really a good idea to do so? If it is so, does that mean that I am actually doing the right thing all along?

No, it is not a good idea to couple classes. It creates a mess that when you have to update one of the classes, you must usually update the other. Minimize such dependencies!
And if you need them, try to keep them to the public interface.