Review Request: Constifying two methods in KCoreConfigSkeleton

Aaron Seigo aseigo at kde.org
Wed Aug 5 22:42:10 BST 2009


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


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.

- Aaron


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