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 12:52:17 GMT
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).

Ross


Mime
View raw message