I understand your point about the code having to be descriptive about why the clause is not present, which is why I suggested using checkstyle, as it allows for configuration of a comment which skips that check.
However, IMHO your point is valid about having an assertion/error such that future changes don't break the logic. Liv Liviu Tudor E: [email protected] C: +1 650-2833815 (USA) M: +44 (0)7591540313 (UK, Europe) W: http://about.me/liviutudor Skype: liviutudor I'm nobody, nobody's perfect -- therefore I'm perfect! On 05/09/2012 13:02, "sebb" <[email protected]> wrote: >Seems to me there are several reasons why the default case may have >been omitted: > >1) It was accidentally omitted. >In this case, add the required clause. > >2) It was omitted because nothing needs to be done. >In this case, this needs to be documented; the easiest way is to add: > >default: > break; // nothing needs to be done here > >3) It was omitted because "it cannot happen" >In this case, add an assertion or throw an error. >This is so that a future change which invalidates the assumption is >not overlooked, but is caught. > >It's important that the person reading the code knows that the switch >block is complete. >So fixing the problem by changing CheckStyle or Findbugs configuration >files is not sufficient; there must be at least a comment in the code. > >On 5 September 2012 01:04, Liviu Tudor <[email protected]> wrote: >> Hi guys, >> >> I am looking at this from a different perspective: the same check can be >> performed using checkstyle >> >>(http://checkstyle.sourceforge.net/config_coding.html#MissingSwitchDefaul >>t) >> as well as FindBugs. So if this is a valid case where there is no need >> for a default branch, we could perhaps simply use a configured >>checkstyle >> comment to instruct checkstyle that this check doesn't need to be >> performed in this case? >> >> If this definitely needs FindBugs, I know that FindBugs supports >> annotations, so it might be worth looking into that, though I don't know >> off the top of my head if there is one available for default missing in >>a >> switch. >> >> >> Liv >> >> Liviu Tudor >> >> E: [email protected] >> C: +1 650-2833815 (USA) >> M: +44 (0)7591540313 (UK, Europe) >> W: http://about.me/liviutudor >> Skype: liviutudor >> >> I'm nobody, nobody's perfect -- therefore I'm perfect! >> >> >> >> >> >> >> >> On 04/09/2012 16:02, "Gilles Sadowski" <[email protected]> >> wrote: >> >>>> > > >>>> > > FindBugs can give warnings like: >>>> > > >>>> > > Switch statement found in >>>> > > org.apache.commons.codec.binary.Base32.decode(byte[], int, int, >>>> > > BaseNCodec$Context) where default case is missing >>>> > > >>>> > > In this case for [codec], it looks like the code was carefully >>>> > constructed >>>> > > and that no default clause is needed. >>>> > > >>>> > > In these cases for any component, this FindBugs issue feels like a >>>>style >>>> > > issue, is it worth changing the code to add a default clause like: >>>> > > >>>> > > default: >>>> > > // ok to fall through for other values. >>>> > > break; >>>> > > >>>> > > Or does this feel like noise to you all? >>>> > >>>> > In Math, there is this kind of code: >>>> > ---CUT--- >>>> > switch (method) { >>>> > case ILLINOIS: >>>> > f0 *= 0.5; >>>> > break; >>>> > case PEGASUS: >>>> > f0 *= f1 / (f1 + fx); >>>> > break; >>>> > case REGULA_FALSI: >>>> > // Detect early that algorithm is stuck, instead >>>>of >>>> > // waiting >>>> > // for the maximum number of iterations to be >>>>exceeded. >>>> > if (x == x1) { >>>> > throw new ConvergenceException(); >>>> > } >>>> > break; >>>> > default: >>>> > // Should never happen. >>>> > throw new MathInternalError(); >>>> > } >>>> > ---CUT--- >>>> > >>>> > >>>> What about the opposite case: >>>> >>>> We do not care about the other values than the ones in each switch >>>>case. >>>> >>> >>>Wouldn't adding an empty "default" be enough to mean that (and silence >>>FindBugs)? >>> >>> >>>Regards, >>>Gilles >>> >>>--------------------------------------------------------------------- >>>To unsubscribe, e-mail: [email protected] >>>For additional commands, e-mail: [email protected] >>> >> >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] >> For additional commands, e-mail: [email protected] >> > >--------------------------------------------------------------------- >To unsubscribe, e-mail: [email protected] >For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
