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 11:56:31 GMT
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, 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

http://svn.apache.org/viewvc?rev=428699&view=rev

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.

> 
> 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(). 

> 
> 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.

> 
> > 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. Raising an exception cost performance
(more then simple tests) and it should be prevented to raise the
exception in the first place (if possible). 

Further catch (FileNotFoundException e) is only getting the
FileNotFoundException and do not handle (ignore) all others. 

+        } catch (Exception e) {
+               getLogger().error("Opps, something went wrong.",e);
+        }

Let you see the Exception that causes the failure and fix it faster.

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