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