4

I'm trying to modify code I found, but I'm blocked by my lack of understanding of the purpose, importance, and/or relevance of this virtual operator:

  1. Can someone provide insight as to why this operator is necessary or useful?
  2. Am I right in thinking it takes parentItem(), rect_, and resizer_ as parameters, then modifies the value of resizer_?

Constructor in .h:

virtual void operator()(QGraphicsItem* item, const QRectF& rect) = 0;

Call in .cpp:

(*resizer_)(parentItem(), rect_);

Trimmed context for the constructor for reference:

class SizeGripItem : public QGraphicsItem
{
    private:

        class HandleItem : public QGraphicsRectItem
        {
            public:
                HandleItem(int positionFlags, SizeGripItem* parent);

            private:    
                SizeGripItem* parent_;
        };

    public:
        class Resizer
        {
            public:
                virtual void operator()(QGraphicsItem* item,
                                        const QRectF& rect) = 0;
        };

        SizeGripItem(Resizer* resizer = 0, QGraphicsItem* parent = 0);
        virtual ~SizeGripItem();

    private:
        void doResize();
        QRectF rect_;
        Resizer* resizer_;
};
Graeme Rock
  • 459
  • 4
  • 14
  • looks like function object. – Pavan Chandaka Nov 02 '16 at 16:37
  • It looks like `Resizer` is a way of having a callback so that the client of `SizeGripItem` can have access to the `rect_` member when one of the `set_x_` member functions (which call `doResize()`) are called. – quamrana Nov 02 '16 at 18:40

2 Answers2

4

The Resizer is a broken attempt at a polymorphic functor (function object). Such an idiom was useful before C++11. It's broken because such functors are useless without a virtual destructor. It should have looked as follows:

class Resizer {
public:
  virtual void operator()(QGraphicsItem* item, const QRectF& rect) = 0;
  virtual ~Resizer() {}
};

Such objects are callable:

void invokeResizer(Resizer * resizer, QGraphicsItem * item, const QRectF & rect) {
  (*resizer)(item, rect);
}

The above will execute the method operator()(QGraphicsItem*,const QRectF&) on the resizer object.

In modern code, instead of such hacks, one should use std::function<void(QGraphicsItem*, const QRectF &)>.

Community
  • 1
  • 1
Kuba hasn't forgotten Monica
  • 88,505
  • 13
  • 129
  • 275
  • Technically, it should be safe to not have a virtual destructor if you have nothing to destruct. – dtech Nov 02 '16 at 17:45
  • 3
    @ddriver That class is polymorphic and abstract: it's **meant** to be overloaded. The absence of a virtual destructor is an error in each and every such class. There's no point to using a functor (instead of a function pointer) if the functor can't hold state. Since a functor can hold state (it's an object for a reason!), it must have a virtual destructor. No ifs nor buts about it. In C++, a functor is an idiom for a callable that can hold state. If you want to force a stateless callable, use a function pointer: that's an idiom that clearly indicates "no state allowed". – Kuba hasn't forgotten Monica Nov 02 '16 at 18:42
  • It is in theory undefined behavior, but in practice what ends up happening is the destructor for the type pointer is being called, that would be a destructor for a base class. If the derived adds no extra members and just overrides functionality then no members will remain non-destructed. Freeing the memory is not an issue either, as the fragment size is usually stored first thing in the fragment itself. Overloading does not necessary imply adding members. It can override functionality, or add functionality, in both cases running the base instead of derived destructor will be practically OK. – dtech Nov 02 '16 at 19:04
  • 3
    @ddriver No, it will never be OK. A polymorphic functor is a **contract** between you and your user. By not providing a virtual destructor, you're breaking your end of the contract. Now you might argue that the absence of the destructor specifies that the functor is not meant to have any data members: if so, then why on Earth would you make it a functor and not a function pointer. So yeah, this is definitely broken, and there's nothing theoretical about it. As a user of such an API, I'd be mighty pissed. Polymorphic functors are an idiom, and you're supposed to use it correctly in your design. – Kuba hasn't forgotten Monica Nov 02 '16 at 19:09
  • @ddriver Think of it like this: C++ idioms are no different than idioms in a natural language like English. Suppose someone were to keep saying "add insult to laceration" instead of "add insult to injury". You might chuckle, but it's grating, and it's not correct. Such idioms carry with them the range of expected uses: a context of applicability. To anyone proficient, a polymorphic functor has certain meaning, a certain expectation. A key part of that expectation is that it is stateful in a **useful** manner. Otherwise it'd be a stateless function pointer without a `void*` parameter. – Kuba hasn't forgotten Monica Nov 02 '16 at 19:12
  • It would only be a problem if a derived type introduces new members. If the "state" members are defined in the base class, and the derived only overrides functionality, then running the base class destructor for a derived instance will not leave any members non-destructed. – dtech Nov 02 '16 at 19:33
  • @Kuba I added the virtual destructor as you recommended, and it made a number of warnings I was receiving cease and desist. (Thanks!) Other than that, I see no change in functionality overall - why is this good bookkeeping again? – Graeme Rock Nov 02 '16 at 19:47
  • @GraemeRock The `SizeGripItem` destructor does `delete resizer_;`. Without a virtual destructor, this will not find the destructor of the subclass of `Resizer`, and is undefined behaviour. Avoidance of raw `delete` is another good reason to use `std::function` here. – Oktalist Nov 02 '16 at 22:35
  • @Kuba You are incorrect. The standard makes it clear that virtual destructor is only needed if you new a Derived object and assign it to a Base pointer or reference and later apply delete to the Base pointer. If the Base destructor is not virtual, then it undefined behaviour. The standard is very clear on that. But if you create objects on the stack and not the heap, the Base destructor can be non-virtual and that is fine, – SJHowe Nov 03 '16 at 22:58
  • @SJHowe 1. We're talking of specific code here, and that code does not allow a non-dynamically-allocated instance of `Resizer`. 2. The standard only defines what's legal, not what's a good API. An API that **arbitrarily** restricts the functionality of a polymorphic functor is still broken, even if under such arbitrary restrictions it would not invoke undefined behavior. – Kuba hasn't forgotten Monica Nov 04 '16 at 13:21
3

Regarding point 2, consider this line:

(*resizer_)(parentItem(), rect_);

resizer_ is likely a pointer to an object of an unknown type T, thus *resizer is a reference to an object of the same type T.
If it has a definition for operator() that accepts two parameters having types (let me say) decltype(parentItem()) and decltype(rect_), you can invoke it as it happens in the example.
In other terms, it's equivalent to:

resizer_->operator()(parentItem(), rect_);

It doesn't modify the value of resizer_ at all.

Can someone provide insight as to why this operator is necessary or useful?

Well, it mostly depends on the context and the actual problem it aims to solve.
It's hard to say from a line of code.
If you find it useless, don't use it. That's all.

skypjack
  • 45,296
  • 16
  • 80
  • 161
  • Yes. I am missing two things. One, it is declared as pure virtual, so some where there should be overridden version before usage. And more over these function object types are mostly used as predicate (like we use in STL functions), but here it is used as just normal function. – Pavan Chandaka Nov 02 '16 at 16:56
  • @PAVANCHANDAKA Pure virtual methods are a feature of the language. I suppose it wasn't possible to give a proper definition and the library designer chosen to let the users define it. The other question is questionable, normal functions can be used as predicates, so I don't get the problem. – skypjack Nov 02 '16 at 17:00