I was browsing through the drools source code I am not a fan of seeing catching Throwable in any java code ... I see this in class org.drools.util.ClassUtils: /** * This method will attempt to create an instance of the specified Class. It uses * a syncrhonized HashMap to cache the reflection Class lookup. * @param className * @return */ public static Object instantiateObject(String className) { Class cls = (Class) ClassUtils.classes.get( className ); if ( cls == null ) { try { cls = Class.forName( className ); ClassUtils.classes.put( className, cls ); } catch ( Throwable e ) { throw new RuntimeException("Unable to load class '" + className + "'", e ); } } Object object = null; try { object = cls.newInstance(); } catch ( Throwable e ) { throw new RuntimeException("Unable to instantiate object for class '" + className + "'", e ); } return object; } I believe this masks important errors where the application should not continue to run and it would be preferable to crash... --zoly ------------------------------------------------------------------------ _______________________________________________ rules-dev mailing list rules-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/rules-dev

If no one is agueing for Throwable, I'll update it to Exception then. Mark Zoltan Farkas wrote: > This is my opinion also and is in line with the java best practices... > > Based on javadoc(see class Error and Exception): > > "The class Exception and its subclasses are a form of Throwable that indicates conditions that a reasonable application might want to catch. " > > "An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch." > > In all my apps I want my application to stop on a unrecoverable error (like OOM). However since most libraries have some catch(Throwable) in the code, I always have to use: > > -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=${DUMP_PATH} -XX:OnOutOfMemoryError="kill -9 %p" > > For me as a user of the drools engine as a 3rd party I would expect that Error not be "hidden" inside a RuntimeException. For me Errors are something I cannot recover from... however I might choose to continue the application on a Exception ... > > This is my opinion on this... > > thanks > > --zoly > > > ________________________________________ > From: rules-dev-bounces(a)lists.jboss.org [mailto:rules-dev-bounces@lists.jboss.org] On Behalf Of Geoffrey Wiseman > Sent: Friday, April 04, 2008 2:10 PM > To: Rules Dev List > Subject: Re: [rules-dev] Questionable Exception Handling > > In most cases, I'd say that Errors shouldn't even be caught and rethrown, personally -- something like an OOME doesn't need additional object creation, for instance. > On Fri, Apr 4, 2008 at 1:55 PM, Mark Proctor <mproctor(a)codehaus.org&gt; wrote: > I don't see a problem with Throwable there? We rethrow again, adding in some additional information. I found that just catching Exception didn't work as you could get verification errors and other stuch things. If you look at the context of that that method is doing too, its using reflection to instantiate a class. > > Mark > Zoltan Farkas wrote: > I was browsing through the drools source code > > I am not a fan of seeing catching Throwable in any java code ... > > I see this in class org.drools.util.ClassUtils: > > /** > * This method will attempt to create an instance of the specified Class. It uses > * a syncrhonized HashMap to cache the reflection Class lookup. > * @param className > * @return > */ > public static Object instantiateObject(String className) { > Class cls = (Class) ClassUtils.classes.get( className ); > if ( cls == null ) { > try { > cls = Class.forName( className ); > ClassUtils.classes.put( className, cls ); > } catch ( Throwable e ) { > throw new RuntimeException("Unable to load class '" + className + "'", e ); > } > } > > Object object = null; > try { > object = cls.newInstance(); > } catch ( Throwable e ) { > throw new RuntimeException("Unable to instantiate object for class '" + className + "'", e ); > } > return object; > } > > I believe this masks important errors where the application should not continue to run and it would be preferable to crash... > > --zoly > > ________________________________________ > > _______________________________________________ > rules-dev mailing list > rules-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/rules-dev > > > > _______________________________________________ > rules-dev mailing list > rules-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/rules-dev > > > > _______________________________________________ rules-dev mailing list rules-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/rules-dev

I'll fix in trunk soon. Mark Greg Barton wrote: > I have to agree with Zoltan on this one. Catching > Throwable is ungood. > > --- Mark Proctor <mproctor(a)codehaus.org&gt; wrote: > > >> If no one is agueing for Throwable, I'll update it >> to Exception then. >> >> Mark >> Zoltan Farkas wrote: >> >>> This is my opinion also and is in line with the >>> >> java best practices... >> >>> Based on javadoc(see class Error and Exception): >>> >>> "The class Exception and its subclasses are a form >>> >> of Throwable that indicates conditions that a >> reasonable application might want to catch. " >> >>> "An Error is a subclass of Throwable that >>> >> indicates serious problems that a reasonable >> application should not try to catch." >> >>> In all my apps I want my application to stop on a >>> >> unrecoverable error (like OOM). However since most >> libraries have some catch(Throwable) in the code, I >> always have to use: >> >>> -XX:+HeapDumpOnOutOfMemoryError >>> >> -XX:HeapDumpPath=${DUMP_PATH} >> -XX:OnOutOfMemoryError="kill -9 %p" >> >>> For me as a user of the drools engine as a 3rd >>> >> party I would expect that Error not be "hidden" >> inside a RuntimeException. For me Errors are >> something I cannot recover from... however I might >> choose to continue the application on a Exception >> ... >> >>> This is my opinion on this... >>> >>> thanks >>> >>> --zoly >>> >>> >>> ________________________________________ >>> From: rules-dev-bounces(a)lists.jboss.org >>> >> [mailto:rules-dev-bounces@lists.jboss.org] On Behalf >> Of Geoffrey Wiseman >> >>> Sent: Friday, April 04, 2008 2:10 PM >>> To: Rules Dev List >>> Subject: Re: [rules-dev] Questionable Exception >>> >> Handling >> >>> In most cases, I'd say that Errors shouldn't even >>> >> be caught and rethrown, personally -- something like >> an OOME doesn't need additional object creation, for >> instance. >> >>> On Fri, Apr 4, 2008 at 1:55 PM, Mark Proctor >>> >> <mproctor(a)codehaus.org&gt; wrote: >> >>> I don't see a problem with Throwable there? We >>> >> rethrow again, adding in some additional >> information. I found that just catching Exception >> didn't work as you could get verification errors and >> other stuch things. If you look at the context of >> that that method is doing too, its using reflection >> to instantiate a class. >> >>> Mark >>> Zoltan Farkas wrote: >>> I was browsing through the drools source code >>> >>> I am not a fan of seeing catching Throwable in any >>> >> java code ... >> >>> >>> I see this in class org.drools.util.ClassUtils: >>> >>> /** >>> * This method will attempt to create an >>> >> instance of the specified Class. It uses >> >>> * a syncrhonized HashMap to cache the >>> >> reflection Class lookup. >> >>> * @param className >>> * @return >>> */ >>> public static Object instantiateObject(String >>> >> className) { >> >>> Class cls = (Class) >>> >> ClassUtils.classes.get( className ); >> >>> if ( cls == null ) { >>> try { >>> cls = Class.forName( className ); >>> ClassUtils.classes.put( >>> >> className, cls ); >> >>> } catch ( Throwable e ) { >>> throw new RuntimeException("Unable >>> >> to load class '" + className + "'", e ); >> >>> } >>> } >>> >>> Object object = null; >>> try { >>> object = cls.newInstance(); >>> >>> } catch ( Throwable e ) { >>> throw new RuntimeException("Unable to >>> >> instantiate object for class '" + className + "'", e >> ); >> >>> } >>> return object; >>> } >>> >>> I believe this masks important errors where the >>> >> application should not continue to run and it would >> be preferable to crash... >> >>> >>> --zoly >>> >>> ________________________________________ >>> >>> _______________________________________________ >>> rules-dev mailing list >>> rules-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/rules-dev >>> >>> >>> >>> _______________________________________________ >>> rules-dev mailing list >>> rules-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/rules-dev >>> >>> >>> >>> >>> >> _______________________________________________ >> rules-dev mailing list >> rules-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/rules-dev >> >> > > > > ____________________________________________________________________________________ > You rock. That's why Blockbuster's offering you one month of Blockbuster Total Access, No Cost. > http://tc.deals.yahoo.com/tc/blockbuster/text5.com > _______________________________________________ > rules-dev mailing list > rules-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/rules-dev > > ------------------------------------------------------------------------ _______________________________________________ rules-dev mailing list rules-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/rules-dev