[Development] QConfig update.

Albert Astals Cid albert.astals at canonical.com
Wed Oct 15 09:25:27 CEST 2014


Please do not assert on the library, you're going to make everyone's
program crasheable by just editing a file by hand.

On Tue, Oct 14, 2014 at 7:06 PM, Milian Wolff <milian.wolff at kdab.com> wrote:

> On Tuesday 14 October 2014 13:44:30 Tomaz Canabrava wrote:
> > On Tue, Oct 14, 2014 at 1:35 PM, Milian Wolff <milian.wolff at kdab.com>
> wrote:
> > > On Tuesday 14 October 2014 13:22:51 Tomaz Canabrava wrote:
> > > > People, I'v read everything on the other two e-mails and I'v changed
> > >
> > > quite
> > >
> > > > a few things here and again I ask for some advice.
> > > >
> > > > What I'v done:
> > > >
> > > > QConfig / QConfigGroup / QConfigWatcher combo classes, to be used
> from
> > >
> > > the
> > >
> > > > user
> > > > QConfigIniBackend, to be used internally.
> > > >
> > > > QConfig:
> > > >    uses a 'global' QHash<QString, QConfigIniBackend> for different
> files
> > >
> > > in
> > >
> > > > a way that we don't destruct the info when the object is destroyed,
> but
> > > > reuses the JSON information stored, and parse it to the config
> object.
> > > >
> > > > it has a QConfigGroup 'global' value that can be acced directly via
> > > > .setValue and .value
> > >
> > > Why the global state? A QConfig should be valid for a single file and
> > > constructed on-demand. If you want to share stuff and keep it open,
> adding
> > > something like a KSharedConfig might be a good idea. But again, that is
> > > something that could/should be built on-top of QConfig (imo).
> >
> > so I don't need to open a config file and parse it every time the user
> > created a QConfig object.
>
> Then provide a QSharedConfig or let the user store the QConfig himself, if
> it
> turns out to be a performance-bottleneck for him. If you add global state
> to
> QConfig, you'll need synchronization or otherwise you are doomed in a
> multithreaded application.
>
> Also: If you add a cheap JSON cache, is it really worth to optimize at all?
>
> Furthermore: If you keep the QConfig in memory all the time, you'll
> probably
> end up duplicating the stuff as soon as you add the fancy high-level
> interface
> on-top. QConfig will operate on JSON after all and you'll incur a
> conversion
> penalty whenever you read a value from it. The high-level interface should
> (hopefully) read the values once and store it internally in the proper
> type.
> If you don't do this and always read from QConfig, you'll end up with
> issues
> in this pattern:
>
> void HighLevelConfig::setFoo(quint64 foo)
> {
>   if (foo == m_foo) {
>     return;
>   }
>   m_foo = foo;
>   emit fooChanged(foo);
> }
>
> If this would read from QConfig, instead of directly comparing to the
> "quint64
> m_foo" member, you'll get rounding errors etc. pp. This, the more I think
> about it, is actually a big issue with a JSON cache in general. .ini does
> not
> have that issue as the byte-array gets interpreted based on the type you
> pass
> along (a kind of duck-typing). It might mean that you'll have to store
> everything as a string in JSON to prevent a loss of data when storing e.g.
> std::limits<quint64>::max() (note: JSON only knows double). But storing
> everything as a string looses the difference between
>
> foo="1"
>
> and
>
> foo=1
>
> Afaik this issue has not yet been discussed, has it?
>
> > > > QConfig and QConfigGroup *does not* support setting a default value
> on
> > >
> > > the
> > >
> > > > getter, I know that this is used in a lot of places but this can
> cause
> > > > inconsistencies:
> > > >
> > > > Main.cpp
> > > >
> > > > QConfig c;
> > > > c.value("window-width", 800);
> > > >
> > > > MainWindow.cpp
> > > > c.value("window-width", 1200);
> > > >
> > > > so we must create a better way for that. for now, I simply removed
> the
> > > > possibility for that, we can only do
> > > >
> > > > c.value("window-width");
> > >
> > > And how do you check whether window-width was read or not? What do you
> > > return
> > > on error? A default-constructed value? What type does the value have
> for
> > > that
> > > matter?
> > >
> > > Imo, this makes this API extremely inconvenient. Yes it's possible to
> do
> > > an
> > > error, but that is life. You should only add such error-prevention
> stuff
> > > into
> > > the high-level schema stuff, not into QConfig itself.
> > >
> > > Without the ability to provide a default value, one can never figure
> out
> > > whether `c.value("some-int") == 0` means the value could not be read
> and a
> > > default should be used, or whether the value is 0.
> >
> > Since I wanted to add schema-validation on the low level stuff, this
> > wouldn't be an issue,
> > 'value couldn't be read' would cause an assert.
> > but I can postpone this for later.
>
> Most applications, (all KDE applications btw), would then assert.
> Initially,
> no configuration file is available after all.
>
> And again: Please do not overdesign QConfig. KISS! Add fancy
> schema-validation
> and stuff on-top of that. Don't remove something as essential as reading a
> setting with a default value fallback mechanism!
>
> Bye
> --
> Milian Wolff | milian.wolff at kdab.com | Software Engineer
> KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
> Tel. Germany +49-30-521325470, Sweden (HQ) +46-563-540090
> KDAB - Qt Experts - Platform-independent software solutions
>
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20141015/999ce866/attachment.html>


More information about the Development mailing list