[Development] QConfig update.

[Milian Wolff]

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