[Kde-games-devel] Krazy Issues

[Frederik Schwarzer]

On Monday 08 December 2008 18:53:54 Albert Astals Cid wrote:
> A Dilluns 08 Desembre 2008, Matthew Woehlke va escriure:
> > Dmitry Suzdalev wrote:
> > > On Sunday 07 December 2008 14:42:03 Frederik Schwarzer wrote:
> > >> While looking at some of the issues in libkdegames (for learning
> > >> reasons), I created the attached patch. Does somebody want to
> > >> have a look at it?
> > >
> > > Looks nice to me, except that this change:
> > >
> > > -	foreach(KGameDifficulty::standardLevel level, m_standardLevels) {
> > > +	foreach(const KGameDifficulty::standardLevel &level, m_standardLevels)
> > > {
> > >
> > > It's not really an optimization, because standardLevel is a enum, and
> > > passing it as a const reference doesn't bring any improvement - it'll be
> > > optimized by compiler.
> > >
> > > Well, no harm either.
> >
> > IMHO simple data types should *not* be tossed around const& :-). (Did
> > Krazy really suggest this change? If so I'd say that should be filed as
> > a bug against Krazy...)
> 
> Krazy is a perl script, can not know this is an enum unless you make it a full 
> C++ parser.
> 
> Albert
> 
> >
> > I'm assuming of course that Dmitry is right about it being an enum, as I
> > didn't actually check.
> >
> > Also, in kgame/dialogs/kgamedialogconfig.cpp, why only replace QLineEdit
> > with KLineEdit? Maybe it would be better to do as the TODO suggests and
> > use KIntNumInput instead?

Well, I' currently quite busy with a study project and only look at
Krazy stuff in between for recreational learning about common
mistakes and such. Since I am not that familiar with C++/Qt yet,
I refrain a bit from "real" changes. :)

Anyway, I excluded the enum reference stuff from the patch. Is the
rest save to commit?

Regards