The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

design review

Could you please comment on the following design approach:

I have various types of objects to keep track of in my application. Some of them are similar, some are not. The commonality for all of them is that they all need to respond to certain types of events. Here is how I thought I could implement this.

I create a unique abstract class per event type to which objects respond. These abstract event classes have pure abstract methods which need to be implemented by objects that need to respond to the event type.

class IFlip { virtual Flip() = 0; }  // flip event
class IRoll { virtual Roll() = 0; }  // roll event
class IRotate { virtual Rotate() = 0; } // rotate event

I also create an abstract base class, CBase, which is the base class for all objects that need to respond to the events above.

class CBase
{
    //note: one getXXX() method per abst event class
    virtual *IFlip getIFlip() { return 0; }
    virtual *IRoll getIRoll() { return 0; }
    virtual *IRotate getIRotate() { return 0; }
    //...
}    

Each object class derives from CBase and from the abstract event classes that it supports. For any event class it supports, it also overrides the getXXX() method in CBase. This way when the object is accessed, the accessor can determine if this object supports the event or not.

// Obj1 supports only IFlip
class Obj1 : CBase, IFlip
{
    virtual *IFlip getIFlip() { return self; }
    Flip() {}
}

// Obj2 supports IFlip and IRoll, but not IRotate
class Obj2 : CBase, IFlip, IRoll
{
    virtual *IFlip getIFlip() { return self; }
    virtual *IRoll getIRoll() { return self; }
    Flip() {}
    Roll() {}
}


All objects reside in a single container and they are accessed via CBase*. The accessors (users of objects) will only need to relate to the objects based on event types. The accessors don't need to or want to know how an object handles the event types internally. All they know are the object ID (to get the object from the container) and the event type. They first get a CBase* to the object from the container and then call the CBase getXXX() method based on the event they are interested in. They either get a valid pointer or a NULL pointer based on whether the object supports that event or not.

When it comes to adding more event classes to this hierarchy, only CBase needs to change to add a getXXX() method for the new abstract event class. The CBase-derived classes don't have to change unless they are interested
in the new event type.

If you made it this far, thank you for reading and please let me know what you think of this design approach. Any suggestions are welcome.
outback
Wednesday, October 11, 2006
 
 
I would look up the Observer Pattern.  What you have is very similar, but that pattern may help with implementation details.

Basically, the problem you could run into is Inheritance.  If you need to subscribe to an event and *not* be a CBase.

I would try this variation:  Make the CBase class have events, any anybody who is interested in that event could just subscribe to it.  When you add new events, those objects that need the event can just subscribe to it.  As you add new events, nobody is impacted, unless they want to be.
Another Anonymous Coward
Wednesday, October 11, 2006
 
 
1) Because of your CBase all objects implement a trivial version  of all operations. That creates dependencies into interfaces which may not be in use.
2) It's probably better if you implement an observer here. Like suggested before.
3) Class / interface names are not suggestive of what the class / interface actually does. Same for methods and comments.

How's this;

class IFlipEventSource {
  virtual fireFlipEvent() = 0;
  virtual registerListener(IFlipListener& listener) = 0;
  virtual unregisterListener(IFlipListener& listener) = 0;
}

class IRollEventSource {
  virtual fireRollEvent() = 0;
  virtual registerListener(IRollListener& listener) = 0;
  virtual unregisterListener(IRollListener& listener) = 0;
}

class IRotateventSource {
  virtual fireRotateEvent() = 0;
  virtual registerListener(IRotateListener& listener) = 0;
  virtual unregisterListener(IRotateListener& listener) = 0;
}


class IFlipListener { virtual onFlip() = 0; } 
class IRollListener { virtual onRoll() = 0; } 
class IRotateListener { virtual onRotate() = 0; }

// Obj1 listenes only to flip events
class Obj1 : IFlipListener
{
    onFlip() { /* do stuff */ }
}

// Obj2 listens to flip and rol events, but not rotate
class Obj2 : IFlipListener, IRollListner
{
    onFlip() { /* do stuff */ }
    onRoll() { /* do stuff */ }
}

Then the main code:

void main(void) {
  IRotateEventSource* pRotator = createRotator();
  IFlipEventSource* pFlipper = createFlipper();
  IRollEventSource* pRoller = createRoller();

  Obj1 obj1;
  Obj2 obj2;

  pFlipper->registerListener(obj1);
  pFlipper->registerListener(obj2);

  pRoller->registerListener(obj1);
  pRoller->registerListener(obj2);

  pRotator->registerListener(obj1);

  while(true) {
      switch(getEvent()) {
        case DONE_EVENT:
            delete pFlipper; delete pRoller; delete pRotator;
            exit(0);
        case FLIP_EVENT: pFlipper->fireFlipEvent(); break;
        case ROLL_EVENT: pRoller->fireRollEvent(); break;
        case ROTATE_EVENT: pRotator->fireRotateEvent(); break;
      }
  }
}
Dino Send private email
Wednesday, October 11, 2006
 
 
IFlip, IRoll, IRotate aren't the events themselves. They are the interfaces that the controller would use to access the data objects when that particular event arrives. I think this wasn't clear in my first description.

Let's say one of the views triggered a "rotate" event on object ID#15 (views only know about object IDs). Then the controller would access object #15 from the container via a CBase* pointer and it would obtain an IRotate pointer to object #15 and manipulate the object data via the IRotate interface. IRotate interface is such that it has everything necessary to manipulate the data for a rotate event.

The design attempts to separate the data type from the event type. I could have various types of objects, but the controller doesn't need to know specifically what type they are. All the controller knows is the event and how the data will be manipulated based on that event.

I think you'll all agree with me that the observer pattern does not apply here.


> Basically, the problem you could run into is
> Inheritance.  If you need to subscribe to an event and
> *not* be a CBase.

The clarification for this is that data objects don't subscribe to events. The controller does.. and when the event arrives, it uses the relevant event interface to manipulate the data. The event interfaces are for standardizing how the data is manipulated based on a given event, so that the specific type of data doesn't need to be known.

> 1) Because of your CBase all objects implement a trivial version  of all operations. That creates dependencies into interfaces which may not be in use.

If I understand you correctly, you are saying that each object class ends up implementing (inheriting) the
getXXXX() methods whether it implements that interface or not. That is one downside of this approach and I am not entirely sure how to improve it. If you have any ideas I am all ears.

On the other hand, CBase does not implement IFlip, IRoll etc.. CBase is just a gateway to the event interface of the data object *if* the data object actually implements that interface. CObj1, CObj2, etc are the ones implementing the event interface if need be.
outback
Wednesday, October 11, 2006
 
 
"This way when the object is accessed, the accessor can determine if this object supports the event or not."

Instead of the accessors checking to see if an object supports a particular event, have the object perform the event directly. Then you won't need individual getXXX() methods. See the IShape example here: http://www.tek271.com/articles/pood/PrinciplesOfOOD.java.html
dev1
Thursday, October 12, 2006
 
 
How do you get rid of the unwanted dependencies? For example:

class IBase {}
class IFlip { virtual void flip() = 0; }
class IRoll { virtual void roll() = 0; }
class IRotate { virtual void rotate() = 0; }

class CObj1 : IBase, IFlip, IRoll {
  virtual void flip() {...}
  virtual void roll() {...}
}

class CObj2 : IBase, IRoll, IRotate {
  virtual void roll() { ... }
  virtual void rotate() { ... }
}

void main(void) {
  IBase* pObj1 = new CObj1();
  IBase* pOjb2 = new CObj2();

  try {
      dynamic_cast<IFlip*>(pObj1)->flip();
      dynamic_cast<IRoll*>(pObj1)->roll();
      // this will throw an exception
      dynamic_cast<IRotate*>(pObj1)->rotate();
  } catch (exception& e) {
      cout << "Exception: " << e.what();
  }

  try {
      dynamic_cast<IRotate*>(pObj2)->rotate();
      dynamic_cast<IRoll*>(pObj2)->roll();
      // this will throw an exception
      dynamic_cast<IFlip*>(pObj2)->flip();
  } catch (exception& e) {
      cout << "Exception: " << e.what();
  }

  delete pObj1;
  delete pObj2;
}

With this you get your base class from which you cross cast into the interface you need. You need to check for exception at cross casting time though (this is somewhat what COM does).

Another way to break things down is to delegate implementation, similar to what you did. Use the bridge pattern. This is good especially if you want to change how you process events at runtime.

enum EventType {
  FlipEvent,
  RollEvent,
  RotateEvent,
}

class CBase {
    EventHandler& getEventHandler(EventType eventType) { ... }
}

class EventHandler {
    virtual void handleEvent() = 0;
}

class FlipEventHandler : EventHandler {
      virtual void handleEvent() {
            onFlipEvent();
      }
      virtual void onFlipEvent() = 0;
}

class RollEventHandler : EventHandler {
      virtual void handleEvent() {
            onRollEvent();
      }
      virtual void onRollEvent() = 0;
}

class RotateEventHandler : EventHandler {
      virtual void handleEvent() {
            onRotateEvent();
      }
      virtual void onRotateEvent() = 0;
}

class RollEventHandler1 : RollEventHandler {
      virtual void onRollEvent() {...}
}

class RollEventHandler2 : RollEventHandler {
      virtual void onRollEvent() {...}
}

void main(void) {
 Event& event = getEvent();
 EventHandler& handler = getEventHandler(event.getType());
 handler.handleEvent();
}

The getEventHandler could return an instance of any RollEventHandler1 or RollEventHandler2 for RollEvents.

The bridge here is made by the EventHandler interface, with dependencies reversing direction around it (they don't go from CBase to RollEvent anymore, but now CBase -> EventHandler <-RollEventHandler <- RollEventHandler1&2 )


I hope this answers your question.
Dino Send private email
Thursday, October 12, 2006
 
 
You may also find some useful stuff on my blog (although I know it is hard to digest).  Click on my name below.
Dino Send private email
Thursday, October 12, 2006
 
 
Dino

Thank you for your suggestion. I don't think the bridge pattern is quite right for what I am doing, but it gave me an idea. I can improve the design like this: Instead of having as many getXXX() methods in CBase as there are events, I could have a single gethandle() like this:

CBase
{
  virtual void* getHandle(EventType type) = 0;
}

Then the object classes implement it like this:

class CObj1 : CBase, IFlip
{
public:
  void* getHandle(EventType type)
  {
    if (type == FlipEvent)
    {
    printf("Flip support\n");
        return (IFlip *) this;
    }

    printf("Not supported!\n");
    return 0;
  }
}

This way, when I add new event types, CBase doesn't have to change at all. This would prevent the entire CBase derived classes to have to be recompiled. Only the object classes that need the extra events need to be modified and recompiled. This is starting to look like COM.

Btw, is it ok to use void* like this?
outback
Friday, October 13, 2006
 
 
I never had any issues with void *  and then casting heaven into hell. Weak typing makes certain things a lot simpler. The price is, you have to be careful with it "Use dynamic_cast and catch for exceptions").

In fact this is what Joel is preaching for, isnt' it? A way of managing casting to / from void *.

The other choice is to have a base class for EventHandlers. It's more code, not to complex though. It may make things clearer.

PS If you use void* make sure you comment this whole thing clearly. Think of the maintainer who's going to pick up the code in 6 months.
Dino Send private email
Friday, October 13, 2006
 
 
I realize the following is a no no.

int i = 500;
void *pd = &i;
double d = *(double *)pd;

But in my case, I think the use of void* is safe since getHandle() is being asked to return a particular type of pointer. The code asking for the pointer and the pointer being returned are identical. There is no casting between different types of pointers. Isn't this how the new operator works anyway?
outback
Friday, October 13, 2006
 
 
The dynamic_cast will throw an exception if the object cannot cast. It needs runtime type information enabled in the compiler. And of course, it's a bit slower.

The thing is, with old casting, you cannot tell if you're casting the right thing or not

int i = 50;
void * pVar = (void*)&i;
double d = *(double*)pVar;

This may be useful if you're writing a fp manipulator in C. Anyway

double d = * dynamic_cast<double>(pVar);

should throw an exception. Double check, I'm a bit rusty in C++. Or it may not even compile and require a reinterpret_cast (i'm not sure).
Dino Send private email
Friday, October 13, 2006
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz