On Wed, Sep 5, 2012 at 4:02 PM, sebb <seb...@gmail.com> 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. >
Good guideline Sebb. Now, we need to circle back to our specific [codec] cases... :) Gary > > On 5 September 2012 01:04, Liviu Tudor <liviu.tu...@gmail.com> 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#MissingSwitchDefault) > > 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: liviu.tu...@gmail.com > > 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" <gil...@harfang.homelinux.org> > > 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: dev-unsubscr...@commons.apache.org > >>For additional commands, e-mail: dev-h...@commons.apache.org > >> > > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > -- E-Mail: garydgreg...@gmail.com | ggreg...@apache.org JUnit in Action, 2nd Ed: <http://goog_1249600977>http://bit.ly/ECvg0 Spring Batch in Action: <http://s.apache.org/HOq>http://bit.ly/bqpbCK Blog: http://garygregory.wordpress.com Home: http://garygregory.com/ Tweet! http://twitter.com/GaryGregory
