forrest-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thorsten Scherler <thorsten.scher...@wyona.com>
Subject Re: Do not raise exception when you can prevent it (was Re: svn commit: r428677)
Date Fri, 04 Aug 2006 14:03:00 GMT
El vie, 04-08-2006 a las 08:59 -0400, Tim Williams escribió:
> 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).

Sorry, guys I just forgot to throw the exception (which was my intention
the first place after all) . I changed this.now.

salu2
-- 
Thorsten Scherler
COO Spain
Wyona Inc.  -  Open Source Content Management  -  Apache Lenya
http://www.wyona.com                   http://lenya.apache.org
thorsten.scherler@wyona.com                thorsten@apache.org


Mime
View raw message