[Kde-games-devel] KGameDifficulty API notes and suggestions

[Dmitry Suzdalev]

Hi, fellows!

Nicolas, today I decided to look at KGameDifficulty class you've
written to see if I can use it in games i maintain.

I have a few suggestions/observations. Perhaps you'll find them useful
:)

1. First of all I think that taking KXmlGuiWindow* as a parameter
means hardcoding. I.e. there's no way for user of KGameDifficulty to
place actions where he/she wants - this is hardcoded in
KGameDifficulty. And I think there will be games that would want some
custom setup. OTOH, you may think like "hardcoded place"="standard
look". Perhaps this argument stands. But I personally prefer more
freedom in where to put actions in my game :). So I'd propose to
somehow redesign the class so it a) doesn't depend on KXmlGuiWindow
argument b) returns {Q|K}Action which can be plugged where the game
writer wants. Or maybe this should become KStandardGameAction? Opinions?

2. Constructor which accepts number of difficulty levels seems to have
some magic hidden behind it (and it has it indeed :)), so it isn't
understandable from the *.h file how it'll behave. Suppose I have no access
to .cpp file - here are the questions, immediately arising in my head:
   - how actions will be named if I pass,say, "3"? "1","2","3"?
 Something meaningful ("easy","normal","hard")? Then how big is the
 number i can pass to continue to get meaningful names? What happens
 if I pass 5 or 6 or 7 as number of difficulty levels? How about 10? API
 docs don't reflect that :) And this constuctor looks unnatural
 because of that. I would get rid of it completely in favour of
 QStringList one :).

3. No need to specify "const" for bool,int (and any other non-reference
or built in type) function parameters - they're copied anyway, so
original value can not change :)

4. Function name "createActionsAndMore()" lives some mistery behind
its name for developer that will work on the code you've written
:). Perhaps it is better to replace "AndMore" with what this "More"
means? :) Or just remove it, leaving name like "createActions()". All
in all, reader doesn't know what AndMore means - so he might as well
not know about it at all :-)

5. Is it really needed to put KDEGAMES_EXPORT before each method
declaration? I really don't know this part, not very experienced in
lib exports area :). If this is needed, than I should add this to
KGamePopupItem public methods too, right?

I hope you'll find this notes useful.
I can help you with this class as this is something really common for
many games - well spotted! :)
(but then I'd like to remove tab indentation from it - do you mind this? ;))

What do you, others think about above notes?

Cheers,
Dmitry.