forrest-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Williams" <william...@gmail.com>
Subject Re: Do not raise exception when you can prevent it (was Re: svn commit: r428677)
Date Fri, 04 Aug 2006 12:59:32 GMT
On 8/4/06, Ross Gardler <rgardler@apache.org> wrote:
> Tim Williams wrote:
> > On 8/4/06, Ross Gardler <rgardler@apache.org> wrote:
> >
> >> Thorsten Scherler wrote:
> >> > El vie, 04-08-2006 a las 09:44 +0000, rgardler@apache.org escribió:
> >>
> >> ...
> >>
> >> > I will change this, if nobody objects. This includes removing all try
> >> > catches surrounding the calling code.
> >>
> >> I like to have lots of log messages. Your approach does not change the
> >> amount of code in use because you if becomes and if...else
> >>
> >> Furthermore, testing that the source exists is less efficient than
> >> dealing with the exception when it does not exist (at least in this
> >> case).
> >>
> >> However, I'm not -1 on you changing it, only -0 - go for it if you have
> >> the desire and nobody objects.
> >
> >
> > +1 for positive .exists() testing.
> >
> >> > Further I would like to add this to the coding guidelines, that
> >> > exception should be prevented and not using them for debugging.
> >>
> >> I'm -1 on defining either as the *only* way to do it. In my opinion both
> >> are acceptable in different circumstances.
> >
> >
> > I agree with Thorsten,  we should document this as our exception
> > handling strategy.  I think the lazy exception catching approach is
> > very rarely acceptable vs positively testing for a condition that is
> > perfectly acceptable and expected.  If we're saying that the file is
> > optional, then we should add that logic to the code not just wrap it
> > with exceptions.  I agree with Thorsten to add it to the guidelines,
> > perhaps you'll change your mind after reading?
> >
> > http://www.javaworld.com/jw-08-2000/jw-0818-exceptions.html
> >
> > Performance... Not compelling on a method that gets executed twice in
> > a apps lifetime.  (Twice?  Yeah, I'm not sure why it gets executed the
> > second time)
> >
> > Verbocity... Not compelling.  The added conditionals could still be
> > factored out if desired, but good design trumps verbocity anyway IMO.
> >
>
> Like I said I am not -1 on changing *this* instance.
>
> As I indicate in my reply to Thorsten, I agree in principle with what is
> proposed. I am only -1 on a blanket policy of doing it *one* way that
> does not consider all cases, there are three ways of dealing with any
> one exception, in each case we should choose the most applicable.
>
> Having guidelines to help decide which approach is correct is a good idea.
>
> Having a blanket policy that encourages:
>
> public initialize() throws Exception {
>    try {
>      ...
>    } catch (Exception e) {
>      log...
>    }
> }
>
> Is very bad (note this is exactly what has been done in the ForrestConf
> module).

;) Yeah, I think we might have gone from bad to bad in this case.  The
guidelines that I *thought* Thorsten was encouraging was anticipating
and preventing exceptions.  I am not sure how we went from that good
practice to blindly suppressing all other exceptions. I'm still
supportive of having a guideline that encourages anticipating and
preventing exceptions and goes against that only for calculated
reasonings (e.g. proven performance gain - which is obviously not the
case here).

--tim

Mime
View raw message