G++ would not allow this, take a look before it drives you nuts (example with QPalette)



  • Oddly enough QWidget::palette returns a const &QPalette, but:

    g++ does not allow to call through a constant reference a member function which modifies its instance.

    Which makes sense.

    It will scream upon:
    @some_label->palette().setColor(QPalette::WindowText, QColor(Qt::blue));
    @
    While accepting:
    @(const_cast<QPalette &>(some_label->palette())).setColor(QPalette::WindowText, QColor(Qt::blue));

    @



  • I don't know why QWidget::palette returns a const reference and not a value (probably it's a fine-tuned optimization) -- QPalette is a value based class.

    Either way, you're not supposed to do that. Do this instead:

    @
    QPalette palette = widget->palette();
    palette.setColor();
    widget->setPalette(palette);
    @



  • You will admit it' s a bit contrived.

    I may not be supposed to do this, as you say, however g++ accepted it without
    the slightest warning. Which is quite reassuring.



  • The compiler accepted it, but did it actually work? Was your palette updated, and the rendering of your widget with it? That would suprise me, really. The setPalette method will obviously trigger the update for the widget, because the palette() method is const and is not supposed to change the state anyway.



  • Clearly you read my post a bit too quickly.

    QWidget::palette() is const, yes, but does not modify anything in this process.
    It returns a constant reference to a QPalette whose setcolor() is then called and
    modifies the instance, that' s its job. The const_cast makes this call possible.

    Please note it does work (platform: Linux Fedora 15 x64).



  • No, I think you read my reply a bit too quickly. That the palette is modified I will believe, but what I am wondering is if the widget is actually redrawn when you do this.



  • The QLabel' s palette is modified before the label becomes visible.
    The label is packed in a QVBoxLayout and shows up with the right
    colour when the parent window of the layout is made visible.
    That' s all. Now if you are still skeptical, try it.

    Note: when I said it worked I obviously meant the label showed up with
    the expected colour, not that the palette was presumably modified.



  • [quote author="Quicksort" date="1317656223"]
    While accepting:
    @
    (const_cast<QPalette &>(some_label->palette())).setColor(QPalette::WindowText, QColor(Qt::blue));

    @
    [/quote]

    You're telling g++ to shut up. And it obeys.



  • Well, that' s perhaps why casts are so helpful !
    Be that as it may, I don' t see the point in the three
    steps maneuver where a simple cast does the trick.
    Why would it be safer to create a new palette and
    then use setPalette() than to talk the member
    function into doing what it' s meant for ?

    Regards.


  • Moderators

    [quote author="Quicksort" date="1317675418"]Well, that' s perhaps why casts are so helpful !
    Be that as it may, I don' t see the point in the three
    steps maneuver where a simple cast does the trick.
    Why would it be safer to create a new palette and
    then use setPalette() than to talk the member
    function into doing what it' s meant for ?

    Regards.[/quote]

    You're assuming that there is a QPalette object in the widget itself which the palette() method returns and that you should just be able to operate on that object, i.e., you want an accessor to an internal member. Apparently, in this case, there does happen to be a QPalette object in the widget that can be modified (using a const cast to "break the rules.")

    However, imagine the scenario where, while you assume that there's just an object sitting in there, there might not be. Perhaps the palette() method might pull arbitrary bits and pieces of information from within the object, and store them in some temporary internal working QPalette and return a reference to that. That's a contrived scenario, granted, but it's the root of what data encapsulation is all about. There is a palette() getter and a setPalette() setter because it decouples the inner workings of how the data is stored from the outside user interface.

    By casting palette() as non-const and then modifying the values, you're making a big assumption about how the class works internally. The fact that it does is complete happenstance and is not guaranteed to work now or at any time later, until such time that the interface changes. It's safer to use the three-step method simply because that's the way the class is documented to work. That's the specified interface in the API.



  • You need a const_cast to do what you want while you are using the public API of a third party library. I would clearly call this a serious design failure - at best.

    And as Franzk already mentioned, const_cast is a kind of shut_up_I_know_what_I_do_cast, so I do not see any reason of what to discuss on that topic, and certainly not with such an "alarming" topic. You did not discover anything new nor anything that drives someone nuts.

    Keep calm and hack on.



  • Very well, but it would be way simpler to have a few QWidget
    access functions allowing to safely modify some QPalette settings.



  • And what's wrong with what I said before?

    [quote author="peppe" date="1317657653"]
    @
    QPalette palette = widget->palette();
    palette.setColor();
    widget->setPalette(palette);
    @
    [/quote]



  • With a single call, without all this fuss, Mr Specialist.
    Got it ?



  • There's no need to use that tone.

    It wouldn't be a single call:
    @widget->palette().setSomething();@
    Is already two calls. For one setting you might consider the 'proper' approach boilerplate, but there are more considerations going on here.

    setPalette() is more explicit about the change to the widget palette than palette().setBrush()

    widget->palette().setBrush() would be exactly like having a public variable. The fact that const & is returned is probably just for speed purposes (arguably it shouldn't even be there).

    widget->setPalette() allows widget to properly and immediately react to palette changes, by calling update() for example.

    returning non-const references encourages code like

    @widget->palette() = mypalette;@
    which is generally considered bad coding (unintuitive).



  • Well, I tried your code, and it is exactly as I predicted: it does not work. That is: it works if you change the palette before a widget is shown. If you change it after it has already been shown, the label is not redrawn. That is what I was asking:

    [quote author="Andre" date="1317662722"]No, I think you read my reply a bit too quickly. That the palette is modified I will believe, but what I am wondering is if the widget is actually redrawn when you do this.[/quote]

    Others have already explained why it is not a good idea. Personally, I find hard-to-read const casts more fuss than three clearly readable lines, but that is just me.



  • [quote author="Quicksort" date="1317685193"]With a single call, without all this fuss, Mr Specialist.
    Got it ?[/quote]

    It's nonsense to use that attitude. Write an inline function so you can modify your palette with a single line of code.



  • [quote author="Quicksort" date="1317685193"]With a single call, without all this fuss, Mr Specialist.
    Got it ?[/quote]

    Please calm down your wording. People are trying to explain things to you. There is absolutely no reason to become personally insulting.


  • Moderators

    Why are you claiming this is a g++ issue?

    I would be surprised if other compilers did not issue a similar error: Modifying a const reference is wrong.

    Your "fix" by forcing it to be non-const is rather questionable though: It might or might not work, depending on how the widget is implemented, ruining the encapsulation OO design is all about.



  • ->Franzk:

    As for my inappropriate tone, let me tell you I am a bit tired of the patronizing
    one of the forum' s divas. Now, I just suggested to add a few QWidget
    functions allowing in one programming step to modify some palette settings.
    It would translate in several function calls behind the scenes but would ease
    programmer' s task.

    ->Andre:

    I have clearly stated that the label is not yet visible when this palette setting
    is modified. Since you have established it does not work if the widget is
    already visible, your method is clearly the right one although it remains
    quite contrived due to the lack of QWidget functions doing the job in one
    programming step.

    ->Others:

    Nothing.



  • You gave an opinion, someone else gave an opinion - so what's all this frenzy about here?
    You decided to post on a public forum, so

    • you should be able to ignore or report posts you find improper or
    • you should not post on public forums

    There is a broad consensuns here at the QDN that rude behaviour and personal insults are not tolerated. Period.

    As to your initial post: The compiler warning has been surpressed by an explicit cast. This is the behaviour I would expect. There is a reason const casts should be rarely used.

    As to your suggestion: Rejected. The API already provides the requested functionality and the saving of two lines of code (at best) does not justify this redundancy. This would violate Qts API design principles. As already suggested I would create an inline function.

    However, feel free to add the requested functionality and submit a merge request. This is how such discussions are usually solved here.



  • ->Volker
    -> Lukas Geyer

    Insults ? For a "Got it ?".
    Our defintions of this word must be very different.
    As for my "rude" behaviour, to quote you Mr Geyer,
    it was triggered by the patronizing tone of most
    answers to my posts. However I must admit your
    last reply' s technical remarks (const_cast and API
    design principles)make perfect sense.

    I just had a peek at your profile:
    Congratulaions for having, and for so long, contributed
    to such a great toolkit.



  • ->peppe

    Sorry for my unbecoming remark.

    -> Tobias Hunger (with some delay)

    Mr Hunger,

    I a am an Assembly language x64, APL and C programmer
    but a newcomer to C++ and its design philosophy.
    As for GUI toolkits, the only one I knew about was GTK +.
    I dropped it because of its implementation of signals I don' t
    like, to the say the least.
    So, I was very surprised by g++ (legitimate) reaction
    to my attempt to call setColor through a constant reference
    to a QPalette. Realizing shortly after that it made sense but
    considering I had the "right" to cast this thing. Well, since
    g++ stopped complaining I thought it might be appropriate
    to inform those likely to run across the same kind of problem
    that my trick worked. It was my sole purpose. Now, it does
    infringe QT design philosophy as you and Mr Geyer, among
    others, pointed out.

    Congratulations for your continuing contribution to Qt.



  • Just (pa)trolling past this thread and it's encouraging to see that things are sort of sorted :)



  • Hi Quicksort,

    [quote author="Quicksort" date="1317767499"]
    I a am an Assembly language x64, APL and C programmer
    but a newcomer to C++ and its design philosophy.
    [/quote]

    that's an important information which will put your first post in a completely different context. I'm pretty sure, everyone here thought you were quite familiar with C++ and the concepts. So some of the answers could have looked a bit weird to you.

    Anyways, if you want to read some good stuff on C++ features, I can recommend you the "C++ FAQs":http://www.parashift.com/c++-faq-lite - it's always a good inspiration. Not to forget to leave behind many of the concepts of assembly language or C (including casts!) when switching to C++ :-)



  • Thank you for your friendly post, Volker.
    I will take a close look at these FAQs, I have a lot to learn...
    Now, you might be, the case being, interested in Dr Agner Fog' s
    manual about C++ optimization (from a programmer viewpoint,
    it' s not about compiler writing).
    I encourage everyone to visit his great website http://www.agner.org/.
    You will find there various manuals and (free) software packages.
    This man has an incredible knowledge of microprocessor architectures
    and assembly language optimization. I owe him so much.

    Cheers.


Log in to reply
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.