Review Request: Avoid setting a style that is already set

[Matthias Fuchs]

> On 2010-05-29 21:32:22, Christoph Feck wrote:
> > Oh, and I always wondered about debug messages from Oxygen, even when I do not use it :)
> > 
> > The NULL check is unnecessary, Qt makes sure there is a style set (even if it is a built-in style), it will assert otherwise.
> 
> Matthias Fuchs wrote:
>     Not sure if you mean that Qt uses Q_ASSERT there, but if you mean that then the NULL check HAS to stay:
>     
>     "Q_ASSERT() is useful for testing pre- and post-conditions during development. It does nothing if QT_NO_DEBUG was defined during compilation."
>     Same is true for the c++ assert.
>     
>     Assert is for debugging only!
> 
> Christoph Feck wrote:
>     Matthias, if Qt cannot find any style you have a bigger problem...
>     
>     Qt itself does not handle the cases where style is 0, it just uses style()->pixelMetric() etc. and it would be ridiculous to add a null check everywhere.

That argument is ok, but not that Qt asserts anyway because you cannot be sure what Qt is compiled like on the user's machine.
Might sound like nitpicking but I have seen asserts being used for things they aren't for and causing problems.


- Matthias


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4178/#review5913
-----------------------------------------------------------


On 2010-05-29 20:28:39, Albert Astals Cid wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4178/
> -----------------------------------------------------------
> 
> (Updated 2010-05-29 20:28:39)
> 
> 
> Review request for kdelibs and Olivier Goffart.
> 
> 
> Summary
> -------
> 
> Qt is not smart enough not to avoid loading a style that is already set. This avoids a unneeded creation and deletion of the current style for me. It works for all styles since QStyleFactory::create does
>     if(ret)
>         ret->setObjectName(style);
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdebase/workspace/qguiplatformplugin_kde/qguiplatformplugin_kde.cpp 1132095 
> 
> Diff: http://reviewboard.kde.org/r/4178/diff
> 
> 
> Testing
> -------
> 
> Started an app, it worked, Oxygen destructor was not called like it was before
> 
> 
> Thanks,
> 
> Albert
> 
>