forrest-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ross Gardler <rgard...@apache.org>
Subject Re: Do not raise exception when you can prevent it (was Re: svn commit: r428677)
Date Fri, 04 Aug 2006 13:07:43 GMT
Tim Williams wrote:
> 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).

Cool, I'm happy with that.

Ross


Mime
View raw message