Suspicious code in revision 867140 (Left Items)

[Christoph Bartoschek]

Am Mittwoch 08 Oktober 2008 schrieb André Wöbbeking:
> On Wednesday 08 October 2008, Matthias Kretz wrote:
> > On Monday 06 October 2008 08:48:18 Christoph Bartoschek wrote:
> > > - kdebase/runtime/phonon/kcm/devicepreference.cpp:660
> > >
> > > It is not idiomatic for C++ to include the last element into the
> > > range.
> >
> > Can't say anything to that as I have not read much C++ code iterating
> > over enums.
>
> AFAIK normally you use < or more generic != as condition.
>
> But to define an extra enum value only to avoid using <= sounds like a
> bad advice as you've to put that extra value also into every switch
> block.

Normally enums are not ranges and one cannot iterate over them. Therefore one 
normally does not need an element one after the last one. But by defining an 
operator++ one allows iteration over an enum and one should consider using a 
half-open range for the enum. Apparently one can iterate over 
Phono::Category.

Half-open ranges are a quite powerful concept that is often used within C++. 
The standard containers and algorithms and modern libraries suggest to use 
ranges as soon as they are appropriate.

I see that my adivce is quite exaggerated. However another advice is to 
always have a default case for switch blocks that catches the impossible 
cases. Because the extra value is such an invalid case it would be caught by 
the default case.

On the other hand it is your code and you have to be comfortable with it.

Christoph