KDE/kdevplatform/plugins/subversion

Andreas Pakulat apaku at gmx.de
Sun Jul 29 11:03:48 UTC 2007


On 29.07.07 06:54:16, dukju ahn wrote:
> 2007/7/29, Andreas Pakulat <apaku at gmx.de>:
> > On 29.07.07 04:58:06, dukju ahn wrote:
> > > 2007/7/28, Andreas Pakulat <apaku at gmx.de>:
> > > > On 28.07.07 18:30:14, Dukju Ahn wrote:
> > > > > --- trunk/KDE/kdevplatform/plugins/subversion/svn_revision.h #693699:693700
> > > > > @@ -24,6 +24,7 @@
> > > > >  #define SVN_REVERT   (KDevelop::VcsJob::Revert)
> > > > >  #define SVN_COPY     (KDevelop::VcsJob::Copy)
> > > > >  #define SVN_MOVE     (KDevelop::VcsJob::Move)
> > > > > +#define SVN_CAT      (KDevelop::VcsJob::Cat)
> > > >
> > > > Whats this? Apart from the fact that you didn't commit the change in
> > > > vcsjob and thus broke the build, why these defines? Whats the use case
> > > > for that?
> > >
> > > Because svn has its unique operations that are not counted
> > > by our common interface. In most cases its ok but, think about
> > > "svn switch", "svn info". The VcsJob will not define enums for
> > > these operations, but subversion plugin still needs this.
> > >
> > > So I had no choice but to define job types again.
> >
> > I'm not questioning wether you need to invent new enum values, but I'm
> > questioning the use of the #define's here. #define is _not_ a proper
> > enum and I don't see a reason why you have them and not use
> > KDevelop::VcsJob::Cat and Co directly - or SvnJob::Info, SvnJob::Switch.
> 
> But why is the #define too bad?

Because they are unneeded and IMHO its just the wrong tool for the job.
enum is a far better tool.

> It doesn't do any harm as far as the values are defined correctly.
> What is the benefit if I redefine everything into enum values such as
> SvnJob::commit ..?

For example: proper typing and type checking by the compiler, not
polluting the global namespace because the enum values would be
"namespaced" by the class name in which they are defined.

Andreas

-- 
You'll feel devilish tonight.  Toss dynamite caps under a flamenco dancer's
heel.




More information about the KDevelop-devel mailing list