Destructor overriding

[wysota]

As long as the method is not virtual -- yes.

[Daylight]

As long as the method is not virtual -- yes.

Why? It seems to me that destructor overriding might prevent potential code duplication if one would need to delete arrows in other places. It will also prevent from potential repeated deletion, if one deleted an arrow, but forgot to clean the pointers in endItem() startItem() and objects. Cleaning of the arrow pointers in outer objects must occur every time the arrow is deleted, so why not implement it in destructor?
Overriding of destructor will allow to make Arrow::stratItem() and Arrow::endItem() methods private.

[wysota]

Why?

Why what?

[Daylight]

Why is it better to implement separate DiagramItem::removeArrows() function with explicit pointer cleaning outside of Arrow object (like it is done in Diagram Scene Example)?

[wysota]

I didn't say it was better or worse. Both are acceptable solutions as far as they serve your goal. As long as you don't call a virtual method on oneself from its own destructor, everything is fine. Consider the fact that QGraphicsItem has its own destructor that will remove all its children and remove itself from the scene. It will not detach it from items that are not part of its parent-child relationship. Your destructor has this problem that it an arrow is not attached to an item (e.g. the item is destroyed before the arrow is removed), your program will crash. In the original code an arrow can function without an object attached to it since it is a "stupid" object that doesn't care about its environment.

[Daylight]

I didn't say it was better or worse. Both are acceptable solutions as far as they serve your goal. As long as you don't call a virtual method on oneself from its own destructor, everything is fine. Consider the fact that QGraphicsItem has its own destructor that will remove all its children and remove itself from the scene. It will not detach it from items that are not part of its parent-child relationship. Your destructor has this problem that it an arrow is not attached to an item (e.g. the item is destroyed before the arrow is removed), your program will crash. In the original code an arrow can function without an object attached to it since it is a "stupid" object that doesn't care about its environment.

If the item is destroyed before the arrow is removed, the program will crash anyway, because the other item will try to access destroyed item via Arrow::start/endItem(). It is not possible, though, because every item stores the list of attached arrows and removes them before destruction.
But, youre right, it seems it is better to make arrow a child of this->startItem(), this is more safe. Example's objects architecture seems a bit odd to me...

[wysota]

No, making the arrow a child of any item is not a good idea. Think what happens if you try to rotate or scale such item.

[Daylight]

No, making the arrow a child of any item is not a good idea. Think what happens if you try to rotate or scale such item.

It should be fine. Arrows rendering is treated in Arrow:: paint(), so it should be ok.

[wysota]

It will not be fine. The arrow will be rotated or scaled as well.

[Daylight]

It will not be fine. The arrow will be rotated or scaled as well.

Why would it be? It will be repainted (correctly) each time the parent object is changed. The painting is done with respect to start/endItem() coordinates using the mapFromItem(myStart/endItem, 0, 0).

[wysota]

The painting is done with respect to start/endItem() coordinates using the mapFromItem(myStart/endItem, 0, 0).

Coordinates of the items will not change if they are rotated or scaled. If you counter that then you counter everything GraphicsView stands for. The arrow simply can't be a child of an item if it is supposed to be independent of the item. Besides it is attached to two items and it can't have two parents.

[Daylight]

Coordinates of the items will not change if they are rotated or scaled. If you counter that then you counter everything GraphicsView stands for. The arrow simply can't be a child of an item if it is supposed to be independent of the item. Besides it is attached to two items and it can't have two parents.

From the logical point of view, arrow may be a child of an item it starts from. It is not completely independent, it should always be connected to two items. Arrows can not be moved directly, only by moving start/end Items.
This is from the example:

void Arrow::updatePosition()
{
    QLineF line(mapFromItem(myStartItem, 0, 0), mapFromItem(myEndItem, 0, 0));
    setLine(line);
}

void Arrow::updatePosition()
{
    QLineF line(mapFromItem(myStartItem, 0, 0), mapFromItem(myEndItem, 0, 0));
    setLine(line);
}

To copy to clipboard, switch view to plain text mode