[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Use of const arg in KConfigGroup constructors doesn't work with
From:       Richard Dale <rdale () foton ! es>
Date:       2008-03-05 21:56:54
Message-ID: 200803052156.55072.rdale () foton ! es
[Download RAW message or body]

On Wednesday 05 March 2008 09:02:29 Thiago Macieira wrote:
> On Wednesday 05 March 2008 09:02:49 Lubos Lunak wrote:
> >   No. In fact it is less clear and more error prone. Using 'const' is a
> > normal C++ way and it lets the language to handle constness
> > automatically. The code you posted will not work (compile) if the
> > KConfigBase is already const and it forces the developer to do manually
> > what the compiler can do on its own (and the compiler is not human, so
> > unlike the developer it should not do mistakes :) ). It is rather common
> > to overload methods in C++ based on constness and I'm surprised this is
> > the first time you've run into it.
>
> I'd say that we should have both things:
>
>  KConfigGroup(KConfigBase *master, const QString &group, AccessType mode =
> ReadOnly);
>  KConfigGroup(const KConfigBase *master, const QString &group);
>
> i.e., the second form is enforced read-only, whereas the first can be
> selected. For instance, I could have a read-write KConfigBase * that I
> wanted to create a read-only form from. Casting it into const seems bizarre
> to me.
>
> In order to maintain binary compatibility, we can't have the first form,
> but we have to introduce a third with a non-default argument. And add a ###
> KDE 5 marker.
The attached patch add two extra constructors with a bool 'isConst' argument:

    /**
     * This variant of the constructor is useful for non-c++ languages
     * without const argument overloading.
     *
     * ### KDE 5 make isConst default to true
     */
    KConfigGroup(KConfigBase *master, const QString &group, bool isConst);
    KConfigGroup(KConfigBase *master, const char *group, bool isConst);

Is it ok to commit this to the trunk?

-- Richard




["kconfiggroup_constructor.patch" (text/x-diff)]

Index: kconfiggroup.h
===================================================================
--- kconfiggroup.h	(revision 782149)
+++ kconfiggroup.h	(working copy)
@@ -64,11 +64,22 @@
      * This allows to create subgroups, by passing an existing group as @p master.
      *
      * @p group is the group name encoded in UTF-8.
+     *
+     * ### KDE 5 remove these two constructors
      */
     KConfigGroup(KConfigBase *master, const QString &group);
     KConfigGroup(KConfigBase *master, const char *group);
 
     /**
+     * This variant of the constructor is useful for non-c++ languages
+     * without const argument overloading.
+     *
+     * ### KDE 5 make isConst default to true
+     */
+    KConfigGroup(KConfigBase *master, const QString &group, bool isConst);
+    KConfigGroup(KConfigBase *master, const char *group, bool isConst);
+
+    /**
      * Construct a read-only config group. A read-only group will silently ignore
      * any attempts to write to it.
      *
Index: kconfiggroup.cpp
===================================================================
--- kconfiggroup.cpp	(revision 782149)
+++ kconfiggroup.cpp	(working copy)
@@ -550,6 +550,16 @@
 {
 }
 
+KConfigGroup::KConfigGroup(KConfigBase *master, const QString &_group, bool isConst)
+    : d(KConfigGroupPrivate::create(master, _group.toUtf8(), \
master->isGroupImmutable(_group), isConst)) +{
+}
+
+KConfigGroup::KConfigGroup(KConfigBase *master, const char *_group, bool isConst)
+ : d(KConfigGroupPrivate::create(master, _group, master->isGroupImmutable(_group), \
isConst)) +{
+}
+
 KConfigGroup::KConfigGroup(const KConfigBase *master, const QString &_group)
     : d(KConfigGroupPrivate::create(const_cast<KConfigBase*>(master), \
_group.toUtf8(), master->isGroupImmutable(_group), true))  {



[prev in list] [next in list] [prev in thread] [next in thread]