Tim Williams wrote:
On 8/4/06, Ross Gardler <[EMAIL PROTECTED]> wrote:
Tim Williams wrote:
> On 8/4/06, Ross Gardler <[EMAIL PROTECTED]> wrote:
>
>> Thorsten Scherler wrote:
>> > El vie, 04-08-2006 a las 09:44 +0000, [EMAIL PROTECTED] 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
