Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
Received-SPF: pass (athena.apache.org: domain of sebbaz@gmail.com designates
 209.85.212.43 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CC6BE414.19E7F%liviu.tudor@gmail.com>
References: <20120904230200.GA24856@dusk.harfang.homelinux.org>
	<CC6BE414.19E7F%liviu.tudor@gmail.com>
Date: Wed, 5 Sep 2012 21:02:51 +0100
Message-ID: 
 <CAOGo0VZyn+Q-hguTT5ZZDKgdNxdTw_dQ=wLYX51V0GqsqjdMvQ@mail.gmail.com>
Subject: Re: [all] FindBugs' Switch statement found in class.method where
 default case is missing
From: sebb <sebbaz@gmail.com>
To: Commons Developers List <dev@commons.apache.org>
Content-Type: text/plain; charset=ISO-8859-1

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 <liviu.tudor@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.tudor@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" <gilles@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-unsubscribe@commons.apache.org
>>For additional commands, e-mail: dev-help@commons.apache.org
>>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org