Typical Design Issue

[sajis997]

Hello forum,


I would like to discuss some design issues which may not related directly to Qt, but in some way it is because i am using the graphics view network to get around with this.

I have a class ArrowGraphicsItem (Subclass of QGraphicsItem) as follows:

class ArrowGraphicsItem : public QGraphicsItem
{
public:
 
    /**
      * \param the source graphics item for the arrow
      * \param the destination graphics item for the arrow, by default it is ZERO
      */
    ArrowGraphicsItem(QGraphicsItem *sourceItem,
                      QGraphicsItem *destinationItem = 0);
 
    ~ArrowGraphicsItem();
 
    enum { Type = UserType + UserTypesArrowGraphicsItem };
 
    /**
      * Retruns the type of this class.Necessary for all  QGraphicsItem subclasses
      * \return The type of this class
      */
    int type() const;
 
    /**
      * The bounding rect for the arrow graphics item
      * \return The bounding rect
      */
    QRectF boundingRect() const;
 
................................................................
 
................................................................
 
protected:
 
    virtual QPainterPath shape() const = 0;
 
    void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget);

class ArrowGraphicsItem : public QGraphicsItem
{
public:

    /**
      * \param the source graphics item for the arrow
      * \param the destination graphics item for the arrow, by default it is ZERO
      */
    ArrowGraphicsItem(QGraphicsItem *sourceItem,
                      QGraphicsItem *destinationItem = 0);

    ~ArrowGraphicsItem();

    enum { Type = UserType + UserTypesArrowGraphicsItem };

    /**
      * Retruns the type of this class.Necessary for all  QGraphicsItem subclasses
      * \return The type of this class
      */
    int type() const;

    /**
      * The bounding rect for the arrow graphics item
      * \return The bounding rect
      */
    QRectF boundingRect() const;

................................................................

................................................................

protected:

    virtual QPainterPath shape() const = 0;

    void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget);

To copy to clipboard, switch view to plain text mode 

I have several subclasses which only over-rides the shape() function in their respective subclasses and paint functional definition is always grabbed from default implementation in the ArrowGraphicsItem.


Is that a good design technique ?




Regards
Sajjad

[wysota]

If you paint all items the same way, it would seem all of them have the same shape. What is the point of reimplementing shape() then?

[sajis997]

Yes the shape will be different in each of the sub-classes

[Piskvorkar]

to wysota: Not necessarily. I also sometime use such a design for similar (and simple) items. Each item implements only the boundingRect() and shape() method.
e.g.:

void ArrowGraphicsItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget)
{
    painter->setPen(getPen());
    painter->strokePath(shape());
    // some other stuff (fill path, outline, etc.)
}
 
QPainterPath ThinArrowGraphicsItem::shape() const
{
    // define thin shape here ...
}
 
QPainterPath WideArrowGraphicsItem::shape() const
{
    // define wide shape here ...
}

void ArrowGraphicsItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget)
{
    painter->setPen(getPen());
    painter->strokePath(shape());
    // some other stuff (fill path, outline, etc.)
}

QPainterPath ThinArrowGraphicsItem::shape() const
{
    // define thin shape here ...
}

QPainterPath WideArrowGraphicsItem::shape() const
{
    // define wide shape here ...
}

To copy to clipboard, switch view to plain text mode 

Is there any problem? I don't think it's so ugly design, is it?

[wysota]

Yes the shape will be different in each of the sub-classes

to wysota: Not necessarily. I also sometime use such a design for similar (and simple) items. Each item implements only the boundingRect() and shape() method.
e.g.:

void ArrowGraphicsItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget)
{
    painter->setPen(getPen());
    painter->strokePath(shape());
    // some other stuff (fill path, outline, etc.)
}
 
QPainterPath ThinArrowGraphicsItem::shape() const
{
    // define thin shape here ...
}
 
QPainterPath WideArrowGraphicsItem::shape() const
{
    // define wide shape here ...
}

void ArrowGraphicsItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget)
{
    painter->setPen(getPen());
    painter->strokePath(shape());
    // some other stuff (fill path, outline, etc.)
}

QPainterPath ThinArrowGraphicsItem::shape() const
{
    // define thin shape here ...
}

QPainterPath WideArrowGraphicsItem::shape() const
{
    // define wide shape here ...
}

To copy to clipboard, switch view to plain text mode 

Is there any problem? I don't think it's so ugly design, is it?

How about just having one class with a setShape() method then? What's the point of subclassing?