[Kde-games-devel] Hacking again KScoreDialog

[Parker Coates]

2008/12/15 Albert Astals Cid:
> Hi, the attached patch files bug 177515: High scores list should not
> alphabetize tabs
>
> Do you agree to commit the patch plus all the needed
> setConfigGroupWeights(KGameDifficulty::levelWeights()); ?

Sorry for the late response. I've been away from home and without
network access for the past two weeks.

In general, I think support for non alphabetical sorting is good, and
the implementation seems clean, but I do have some concerns about
consistency in the API. setConfigGroupWeights(...) operates
differently than differently than the similar functions
addLocalizedConfigGroupName(...) and
	addLocalizedConfigGroupNames(...).

Here's a list of problems I see:


- There is no way to set the weight of a single group. A
setConfigGroupWeight(...) function should probably be added.

- The addLocalized... functions inserts new translations into the
classes internal map, while setConfigGroupWeights only allows you to
replace the existing internal map with a new one. Because there is no
way to retrieve the internal map, the class' user must keep a local
copy of the map if they want to make changes to the weights after
originally setting them. I recommend switching to "add" functions
instead of "set" functions.

- The addLocalized... functions map a groupkey to a translated string,
while setConfigGroupWeights maps a weight to a groupKey. For
consistencies sake, I would prefer to see the parameter changed to a
QMap<QByteArray, int>.

- The API documentation doesn't specify what affect the integer weight
has on sorting. Do higher weights come first or last?


I realise that at the moment the above issues don't make much of a
difference as the only current use case is a single call to
setConfigGroupWeights(KGameDifficulty::levelWeights()), but I hope
you'll agree that if we bother adding this functionality, we might as
well do it with enough flexibility to support new uses in the future.

I'd be willing to (quickly) make the changes described above if you
think they make sense. At what point is our API supposed to be frozen
for 4.2?

Parker