View source

You do not have permission to edit this page, for the following reason:

You can view and copy the source of this page:

==Reworking the BibleTime configuration system== ===Existing system=== ====CBTConfig==== Currently there is the CBTConfig class which handles the configuration. It has many enums which are used as arguments in methods (or actually functions, because all are static). There are functions to get values and default values and to set values, about 25 counted together, plus private functions. KConfig is used as a backend. In implementation there are many private getKey functions which have different kinds of default values, like: switch ( ID ) { case bibletimeVersion: return "bibletimeVersion"; Then there are many public getDefault functions: switch ( ID ) { case standardBible: return "KJV"; And of course many get functions: KConfigGroup cg = CBTConfig::getConfig()->group("ints"); return cg.readEntry(getKey(ID), getDefault(ID)); And many set functions: KConfigGroup cg = CBTConfig::getConfig()->group("strings"); cg.writeEntry(getKey(ID), value); This all works but I really think it is unnecessarily complicated. KConfig and QSettings both have systems with which the programmer does not have to care about types. All types can be saved and loaded with same functions. Using a horde of functions (and namespaces) destroys this idea. ====CResMgr==== There is also CResMgr class which handles the values as a treelike namespace structure. There are .h and .cpp files and both have essentially the same code lines, .h for declarations and .cpp for definitions. namespace modules { namespace bible { const QString icon_unlocked = "bible.svg"; This also works but I really think it is very ugly (namespaces are not meant for this) and unnecessarily complicated. Also it's very restricting see the reasoning below, "Actions". ====CProfile==== CProfile class handles saving and loading profiles. Using XML is overkill because the documents are not handled anywhere else besides this class. All information could be saved in a tree structure as key/value pairs. Actually QSettings documentation gives an explicit example for exactly that situation, "Restoring the State of a GUI Application". ===Replacement: BtConfig=== ====The BtConfig class==== As a replacement I give one class. It has very small public interface: class BtConfig { public: enum ItemFlags {NoSave, Save, NoChange}; static void setValue(QString key, QVariant value, ItemFlags flags = NoSave); static QVariant value(QString key); static bool hasValue(QString key); static void initialize(); //... } At first sight this looks impossible. How could such interface replace or at least greatly simplify those three classes above? QVariant is the key (actually the value, but we are speaking figuratively :). In the current system we have to handle all types differently and therefore use enums and countless functions. But Qt has a solution for this: QVariant. It is magic. Without it we must write for example: setIntValue(some_key, 12); setStringValue(some_other_key, QString("hello")); int i = intValue(some_key); QString s = stringValue(some_other_key); But with QVariant we can write: setValue(some_key, 12); setValue(some_other_key, QString("hello")); int i = value(some_key).toInt(); QString s = value(some_other_key).toString(); Now keys are namespaces. We have to write: setValue(CBTConfig::strings::somekeyname, 12); But we could as well write: setValue("somekeyname", 12); Why string keys are better? We will see. ====Actions==== Porting the actions from Qt/KDE3 to 4 was pain because the constructors had changed. There are dozens of actions in the code, spread in many files. Every one of them had to be changed. I wrote one convenience function into one class with which I could reduce the code a bit. But alas, our configuration system did not let make a simple solution. Let's see why. Here is a convenience function (a real one): void initAction(KAction* action, QString text, QString icon, KShortcut accel, QString tooltip, const char* actionName, const char* slot ); And you would call it like this (a real example): initAction( new KAction(ac), i18n("Search in &open work(s)"), CResMgr::mainMenu::mainIndex::search::icon, CResMgr::mainMenu::mainIndex::search::accel, CResMgr::mainMenu::mainIndex::search::tooltip, CResMgr::mainMenu::mainIndex::search::actionName, SLOT( slotSearchModules() ) ); This is not much shorter than any constructor/setters combination and yet you have to write the same code for every Action. What is worse, we have CResMgr::mainMenu::mainIndex::search:: 4 times. Apparently I would like to call it with "CResMgr::mainMenu::mainIndex::search::" once without "icon", "accel" etc. because they are repeated for every action. The shortest possible call would be this: initAction(action, CResMgr::mainMenu::mainIndex::search::, SLOT(slotSearchModules()); But using namespaces forbids it. There is no way to use namespace as an argument. It would be possible with strings: initAction(action, "mainMenu/mainIndex/search", SLOT...); And implementation would look like this: //initAction implementation... action.setIcon(QIcon(CResMgr.value(QString(key) + QString("icon") ))); //... We could make new BtAction class which would inherit QAction or KAction and would have couple of constructors for different situations, basically like this: BtAction::BtAction(QString actionName, QObject* receiver, const char* slot) { if ( CResMgr::hasValue(QString(key) + QString("icon") )) { setIcon(QIcon(CResMgr::value(QString(key) + QString("icon") ))); } //set other properties, connect signal etc. } The idea is that we can define all actions in some centralized place and then construct them automatically. There would also be added benefit that we could make BtAction inherit KAction if it feels necessary and later change it to inherit QAction. We could also make our own BtActionCollection inheriting KActionCollection which indeed is responsible for providing the actions to the shortcut editor. Later we could replace the relevant part of the API with our own implementation. ====Default values==== We want of course provide default values for configuration. It would be easy. We could have file btconfig_defaults.cpp just for that: void BtConfig::initializeDefaults(){ setDefault("mainMenu/mainIndex/actions/search/icon", "icon.svg"); setDefault("mainMenu/mainIndex/actions/search/accel", QKeySequence(), BtConfig::Save); setDefault("mainMenu/mainIndex/actions/search/text", tr("Search in &open work(s)", "Search action in main window bookshelf index") ); //see Qt translation tutorial setDefault("mainMenu/mainIndex/actions/search/tooltip", tr("Search in all open works", "Search action tooltip in main window bookshelf index") ); //... //--------------- replace the CResMgr structures:------------------------------- setDefault("moduletypes/bible/icons/unlocked", "bible.svg", BtConfig::NoChange); //... } "BtConfig::Save" tells that this value can be saved into file, otherwise it would be only in the runtime memory and not saved. It is very easy to make some value saveable e.g. if we want to use icon sets. BtConfig::NoChange tells that the default value may not be changed at all (maybe this is not necessary but it prevents accidents). There might be need for private setDefault function because it may be necessary to revert back to defaults even though there are saved values. For example if the user wants to switch back to default shortcuts. The backend would remember those defaults even after they were overriden. Also when getting a value we should first check if there were a saved/changed value and only after that revert to the default. In the above example there is key string "mainMenu/mainIndex/actions/search/accel". The key tree structure should be standardized. Then we could make the configuration dialog to use the keys to find configurable shortcuts. This may need another new function but basically it would be searching for all actions under some base key. If it has been set as Saveable and Changeable the config dialog would take it to the list of modifiable shortcuts. ====Independent components with Signals and Slots==== It's not necessary to use signals and slots with the new system but it would be possible. Nowadays the Configuration dialog is not complicated and applying the settings calls a slot method in BibleTime class. This is not bad but it makes unnecessary dependencies. BibleTime class doesn't have to know anything about configuration changes, instead BtConfig could send signals when settings are changed and the relevant GUI classes could catch and process them. ====Backend implementation==== Without an easy backend this would be much harder, but we already have QSettings. Using QVariant becomes directly from QSettings. QSettings handles the loading and saving automatically. It is also a solution to config file placement problem. QSettings is overkill if we just want to create a key/value pair runtime and keep it in memory (like icon names etc.). QSettings documentation recommends using QMap<QString, QVariant> for that purpose. So our implementation would have two backends: QSettings for permanent saveable storage and QMap for runtime-only storage. Therefore setValue("key1", "value1", BtConfig::Save); setValue("key2", "value2", BtConfig::NoSave); would put the first pair into QSettings and the second one into QMap. The getter function finds the value from either of these and the caller doesn't have to know where it comes from. Undoubtedly there will be more implementation details but this will still be much more simple than the current solution. ====Drawbacks==== Are there any downsides in this solution? *Using namespaces prevents all spelling mistakes. Using strings is always risky. But QSettings system already takes that risk, so why don't we? **You could still use "const QString"s for all names that are used to access the values. So you can still use the compiler to check your code for spelling mistakes. All the names would be defined in the config header file. What do you think? --mgruner ** After some hard thinking I probably get what you mean. We could for example have a Config:: namespace and inside it some const strings: namespace Config { const char* bibleWindow_actions_nextVerse_accel = "bibleWindow/actions/nextVerse/accel"; } **and then the name could be used instead of the string: getValue(Config::bibleWindow_actions_nextVerse_accel); **It just means that we have to duplicate some text. Is it worth it? Build-time checking is good because it prevents small but annoying bugs, otherwise it's not important. QSettings and KConfig are already built to use only runtime checking and yet they are used in real applications so it should be enough for us too. [[User:Eelik|Eelik]] 16:34, 19 November 2007 (CET) *The user has to check what type the value is. The current system makes sure that it is always correct. I'm not sure how strictly QVariant handles conversions. *I can't say anything about the runtime time/memory penalty of either old or new solution. I suppose it would be negligible though. *QVariant can not handle just any type out there. See the QVariant class documentation. Most important/interesting for us would be: **QString **QStringList **QMap<QString, QVariant> **QList<QVariant> **int **bool **QRect (window geometry) **QFont **QKeySequence (accelerators) **Therefore we have to either break some of our own classes like CLanguageMgr::Language into pieces to save them, make them known to QVariant (see the doc again) or write extra setter/getter functions for them. *Using QVariant essentially means removing strict typing, which can be dangerous. Of course, we will have to use QVariant with QSettings itself, but perhaps our class can use simple overloading (though we'll have to define more functions for the getters). If we only have a few types, would overloading really add much more complexity? Each function may be able to be as simple as checking the type() of the variable and doing the conversion. **This approach might even reduce the complexity, since the function that uses the QVariant will probably need to convert it to its native type anyhow. **This would prevent the user from having to check the type outside of the class. **This approach would make handing our own classes like CLanguageMgr::Language similar to native types from the perspective of code using the class. -jerickson314 ** Yes, that is what I meant by "The user has to check..." above. I can't find a solution to easiness/safety dilemma here but neither have the writers of KConfig or QSettings found it. They follow the easy way, not the safe one. You may be right about adding functions, but I was trying to avoid adding a new function for every type we add to the config data. We have to think about this, checking all different kinds of situations where the config system could be used, and see what would be the best solution in each place. We shouldn't over-engineer but the config system is surely one place where we need flexibility for the future needs. Could this be a place for templates? I'm not familiar enough with them to say for sure. [[User:Eelik|Eelik]] 20:11, 14 November 2007 (CET) ** If we add getters we have not reduced the complexity. We would have more functions and each user should use a specific function anyways. It would not be much easier to use for example getInt() than get().toInt(). [[User:Eelik|Eelik]] 20:11, 14 November 2007 (CET) == Proposal == I (Patrick) will braindump my current thoughts about this here. *Putting a wrapper around QSettings will allow for several nify features, see below. *Do we actually need ephemeral setting storage? (probably not) *Singleton class should be used if we put everything in one config, otherwise a configmanager would be needed, probably overkill *Strings as key. *Put Sessions into that config or separate ones? - Same config. **Separate ones introduce hidingplaces for bugs. **The wrapper class should abstract the access to the current session ** The wrapper class could save a QSet with entries that are saved in sessions. Every access to such elements would automatically put itself in the session if it has to *** Might that cause a performance problem? * BtConfig::getSingleton().getValue("gui/mainwindow/showStuff").value<QString>() * Defaults could be stored in a QHash<QString,QVariant> that is initialized once (and might even be cached to prevent regeneration). *bookmarks should be kept as XML. They can not be represented in QVariants well and are interesting for other applications too. *No type based hierarchy (int, bool, stringlist...) but purpose/origin based. See below. === Configuration layout === Rough layout of Configuration file, this is in no way final. A lot of options are missing and they might be majorly reordered. *gui **mainwindow ***mainwindow options *settings **search **shortcuts ***shortcuts go here **Language **Font **Filters ***global filter options go here *sessions **sessionname ***gui ****mainwindow *****showBookshelf *****showBookmark *****showMag *****... ****BibleWindows *****VerseKey *****Bibles (QStringList) *****useLinebreaks *****useVerseNumbers *****... ****CommentWindows *****similar to BibleWindows ****BookWindows ****DictionaryWindow

Return to Reworking the Configuration System.