forrest-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thorsten Scherler <>
Subject Re: Do not raise exception when you can prevent it (was Re: svn commit: r428677)
Date Fri, 04 Aug 2006 14:11:39 GMT
El vie, 04-08-2006 a las 13:42 +0100, Ross Gardler escribió:
> Thorsten Scherler wrote:
> > El vie, 04-08-2006 a las 12:22 +0100, Ross Gardler escribió:
> > 
> >>Thorsten Scherler wrote:
> >>
> >>>El vie, 04-08-2006 a las 09:44 +0000, 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
> > 
> > 
> >
> > 
> > That is not right. Since I implement the test in the method I reduced
> > the code quite a bit (from 6 try-catch to 1). Regarding the log messages
> > their are still there.
> The code is reduced the way you currently implemented it, yes. But so 
> are the log messages. Now we get "oops something went wrong", whereas 
> before we were told which files loaded OK which were missing etc.

Please review the commit carefully since what you say is not true. I
forgot to throw the exception on the end (what I fixed now) but *All*
other messages "which files loaded OK" are still there!

> >>Furthermore, testing that the source exists is less efficient than 
> >>dealing with the exception when it does not exist (at least in this case).
> > 
> > 
> > That is wrong as well, since source.exits() is implicitly included when
> > using source.getInputStream(). 
> But by putting source exists in *addition* to the 
> source.getInputStream() you do that checking twice - double the work.

It is not about performance at all, I think this thread is showing this.

> >>However, I'm not -1 on you changing it, only -0 - go for it if you have 
> >>the desire and nobody objects.
> > 
> > 
> > No, this is a general issue, that we all need to keep watching. If not,
> > we  will start catching NPE for debugging. That is just bad programming.
> That's a hell of leap in usage patterns. There is no correlation between 
> trapping an IOException and trapping a runtime exception.
> In my view it is correct to trap *all* checked exceptions (i.e. not 
> runtime) and handle them appropriately. If the code can't deal with the 
> situation then it throws a runtime exception which is bubbled up to the 
> user.

...but why provoking instead of prevent them?

> >>>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.
> > 
> > 
> > Well, then this needs discussion. For me it is unacceptable to (ab)use
> > exception for debugging reasons. 
> Agreed and it is for this reason I am not -1 on you making the changes 
> to the properties module.
> However, you go on to support your argument with the point that it is 
> less efficient to raise the exception than it is to prevent the 
> exception happening. This is not always the case.

Actually I just picked up your argument of efficiency. Neither of us can
proof which is more efficient. 

> Imagine a situation when the exception is rarely thrown, e.g. the file 
> exists in 99.9% of projects. In this instance it is more efficient to 
> assume the file exists and to handle the odd occasion it doesn't than it 
> is to test for existence every time.

Disagree, see the link that Tim provided.

> Hence I am -1 on prescribing only one way of handling this situation. 
> I'm OK with having guidelines describing which approach to take in 
> different situations.


Let us try to find consensus on this ASAP because I see this thread
starting again being an end(sense)less discussion.

Thorsten Scherler
COO Spain
Wyona Inc.  -  Open Source Content Management  -  Apache Lenya               

View raw message