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.wo...@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.wo...@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.wo...@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@qt-project.org > http://lists.qt-project.org/mailman/listinfo/development >
_______________________________________________ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development