Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10183 - rbutil settings with static accessors

Attached to Project: Rockbox
Opened by Dominik Wenger (Domonoky) - Saturday, 02 May 2009, 23:52 GMT+2
Last edited by Dominik Wenger (Domonoky) - Saturday, 09 May 2009, 18:59 GMT+2
Task Type Patches
Category Rbutil
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Version 3.2
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

Here is a first attempt to static accessors for the RbSettings Class.

This removes the need to pass settings pointers around. And the settings are always accessible.

   rbutil-static-settings.patch (95.3 KiB) 
 rbutil/rbutilqt/themesinstallwindow.cpp |   21 ++-
 rbutil/rbutilqt/rbutilqt.cpp            |  174 +++++++++++++++-----------------
 rbutil/rbutilqt/uninstallwindow.h       |    4 
 rbutil/rbutilqt/voicefile.h             |    5 
 rbutil/rbutilqt/install.h               |    4 
 rbutil/rbutilqt/installtalkwindow.h     |    6 -
 rbutil/rbutilqt/createvoicewindow.cpp   |   36 ++----
 rbutil/rbutilqt/configure.h             |    5 
 rbutil/rbutilqt/talkfile.h              |    3 
 rbutil/rbutilqt/uninstallwindow.cpp     |   41 +++----
 rbutil/rbutilqt/voicefile.cpp           |    9 -
 rbutil/rbutilqt/install.cpp             |   44 ++++----
 rbutil/rbutilqt/themesinstallwindow.h   |    3 
 rbutil/rbutilqt/installtalkwindow.cpp   |   26 +---
 rbutil/rbutilqt/rbutilqt.h              |    5 
 rbutil/rbutilqt/configure.cpp           |  108 ++++++++-----------
 rbutil/rbutilqt/createvoicewindow.h     |    4 
 rbutil/rbutilqt/base/encoders.cpp       |   49 ++++-----
 rbutil/rbutilqt/base/autodetection.cpp  |    7 -
 rbutil/rbutilqt/base/detect.cpp         |    9 -
 rbutil/rbutilqt/base/tts.cpp            |   81 +++++++-------
 rbutil/rbutilqt/base/rbsettings.cpp     |   93 +++++++++++++++--
 rbutil/rbutilqt/base/encoders.h         |    7 -
 rbutil/rbutilqt/base/autodetection.h    |    4 
 rbutil/rbutilqt/base/detect.h           |    3 
 rbutil/rbutilqt/base/tts.h              |    7 -
 rbutil/rbutilqt/base/rbsettings.h       |   90 +++++++++-------
 rbutil/rbutilqt/talkfile.cpp            |    7 -
 28 files changed, 433 insertions(+), 422 deletions(-)

This task depends upon

Closed by  Dominik Wenger (Domonoky)
Saturday, 09 May 2009, 18:59 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  Improved and commited.
Comment by Dominik Riebeling (bluebrother) - Sunday, 03 May 2009, 15:32 GMT+2
I had a look at that patch and a few questions about it:
- why this split with creating a RbSettings object through the static access functions and accessing them through that object? This seems too complicated to me. Everything we need for this to work are the two QSettings objects and pointers. Why don't you create two static QSettings pointers and make sure those are initialized (with ensureRbSettingsExist()). Then the only thing that needs to get changed is checking that before doing any access handling.
- those "p_" prefixes for private members is weird -- I've never seen that before, and the rest of Rockbox Utility code doesn't do that either. Why? It looks confusing (plus, if you remove the split I mentioned above I don't see any reason for having static and private access functions)
- and for minor nitpicking, platform isn't written with "tt" in english ;-)

Loading...