G++ would not allow this, take a look before it drives you nuts (example with QPalette)
-
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).
-
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. -
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 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. -
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.
-
-
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 GeyerInsults ? 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.