Review Request: Constifying two methods in KCoreConfigSkeleton

Aaron Seigo aseigo at kde.org
Thu Aug 6 14:19:28 BST 2009



> On 2009-08-05 21:42:17, Aaron Seigo wrote:
> > looks good; and afaik the const_cast is the best that can be done in this case while keeping a single implementation. the old method can't be inlined, for instance. :/
> > 
> > as an asid, i wish it was possible to create a KConfigSkeleton with a KConfigBase* object; right now it's either a QString or a KSharedConfig::Ptr (which can only be created with a QString) and that means you can't use it with custom KConfigBase objects (e.g. one that doesn't write to disk). i should take a look at that .. hm.
> 
> Parker Coates wrote:
>     Should I interpret this as a "Ship It"?

from my side, yes. i was waiting for anyone else to comment before clicking Ship It, however :)

oh, one thing that does need to be fixed: @since tags should be added to the new methods. otherwise, afaics, this is good to go.


- Aaron


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


On 2009-08-05 21:00:18, Parker Coates wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1233/
> -----------------------------------------------------------
> 
> (Updated 2009-08-05 21:00:18)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> KCoreConfigSkeleton::isImmuttable and KCoreConfigSkeleton::findItem are non-const methods, even though neither of them touch the internal state of the object. This patch adds const overloads for those two methods.
> 
> For binary compatibility reasons, the existing methods can't be removed. I've added "KDE5 TODO" comments indicating that the non-const versions should be removed in the future. Should they also be marked depreciated? That would seem a bit silly as the compiler automatically selects which of the two versions to call. Should I add a note to the API documentation, or is that really necessary? Is the use of const_cast in the non const versions a safe way to keep only a single implementation? Is there anything else I'm missing?
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kdecore/config/kcoreconfigskeleton.h 1007370 
>   trunk/KDE/kdelibs/kdecore/config/kcoreconfigskeleton.cpp 1007370 
> 
> Diff: http://reviewboard.kde.org/r/1233/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Parker
> 
>





More information about the kde-core-devel mailing list