+1 I recall when this discussion came up during the development of Solder and we all pretty much came to the same conclusion. So you can feel confident there is quite an army in support of this +1 :)
-Dan On Tue, Jan 24, 2012 at 06:36, José Rodolfo Freitas < [email protected]> wrote: > also +1 > > On Mon, Jan 23, 2012 at 7:13 PM, Jason Porter <[email protected] > >wrote: > > > +1 to throwing an exception instead of returning null. > > > > On Mon, Jan 23, 2012 at 03:17, Gerhard Petracek > > <[email protected]>wrote: > > > > > hi christian, > > > > > > we should just wait a bit (to see if someone raises objections) and > then > > we > > > can commit the last suggestion. > > > > > > regards, > > > gerhard > > > > > > > > > > > > 2012/1/23 Christian Kaltepoth <[email protected]> > > > > > > > Hi Gerhard, > > > > > > > > OK, sure. I just wanted to make sure you didn't get me wrong. :) > > > > > > > > Christian > > > > > > > > > > > > 2012/1/23 Gerhard Petracek <[email protected]>: > > > > > hi christian, > > > > > > > > > > i know (/i read it that way). -> the answer is still the same. > > > > > > > > > > regards, > > > > > gerhard > > > > > > > > > > > > > > > > > > > > 2012/1/23 Christian Kaltepoth <[email protected]> > > > > > > > > > >> @gerhard: > > > > >> > > > > >> Sorry, I think my last point was a bit mistakable. Not > > > > >> #getBeanManager() itself can throw NPEs but it can return null > which > > > > >> client code has to check explicitly. If it doesn't, the code will > > > > >> probably fail we NPEs. > > > > >> > > > > >> So my suggestion is to change #getBeanManager() to never return > null > > > > >> and instead throw a meaningful runtime exception just like > > > > >> #getInstance() does it already. > > > > >> > > > > >> Christian > > > > >> > > > > >> > > > > >> 2012/1/23 Gerhard Petracek <[email protected]>: > > > > >> > @IllegalStateException: > > > > >> > +1 (that's what we have already). > > > > >> > > > > > >> > @christian: > > > > >> > i agree and therefore we have the jndi lookup in place. > > > > >> > we have never seen a npe caused by #getBeanManager, however, it > > also > > > > >> > doesn't harm to check it. > > > > >> > > > > > >> > regards, > > > > >> > gerhard > > > > >> > > > > > >> > > > > > >> > > > > > >> > 2012/1/23 Arne Limburg <[email protected]> > > > > >> > > > > > >> >> I am with you here, IllegalStateException should suffice. > > > > >> >> > > > > >> >> Btw. Which Exception will be thrown by CDI.current() in CDI 1.1 > > > when > > > > no > > > > >> >> CDI is available or does it throw none? We should throw the > same > > > > >> exception > > > > >> >> unless it does not exist in CDI 1.0. > > > > >> >> > > > > >> >> Cheers, > > > > >> >> Arne > > > > >> >> > > > > >> >> -----Ursprüngliche Nachricht----- > > > > >> >> Von: Mark Struberg [mailto:[email protected]] > > > > >> >> Gesendet: Montag, 23. Januar 2012 08:19 > > > > >> >> An: [email protected] > > > > >> >> Betreff: Re: [DISCUSS] BeanManagerProvider API > > > > >> >> > > > > >> >> I'd say we should throw an IllegalStateException. > > > > >> >> > > > > >> >> > > > > >> >> IllegalStateException and not BeanManagerUnavailableException > > > > because I > > > > >> >> don't like to have too many custom Exceptions if they don't > carry > > > > >> business > > > > >> >> information. > > > > >> >> This is clearly a non-expected and non-recoverable situation, > > thus > > > a > > > > >> >> RuntimeException is excactly what we need imo. > > > > >> >> > > > > >> >> LieGrue, > > > > >> >> strub > > > > >> >> > > > > >> >> > > > > >> >> > > > > >> >> ----- Original Message ----- > > > > >> >> > From: Christian Kaltepoth <[email protected]> > > > > >> >> > To: [email protected] > > > > >> >> > Cc: > > > > >> >> > Sent: Monday, January 23, 2012 7:19 AM > > > > >> >> > Subject: [DISCUSS] BeanManagerProvider API > > > > >> >> > > > > > >> >> > Hey @all, > > > > >> >> > > > > > >> >> > yesterday I had a deeper look at the BeanManagerProvider > > > > >> >> > implementation and found something that could be improved. I > > > > created > > > > >> >> > DELTASPIKE-56 (see [1]) for this but it turned out that we > > should > > > > >> >> > discuss this topic on the mailing list. > > > > >> >> > > > > > >> >> > I saw that getBeanManager() may return null in some rare > > > > >> >> > circumstances. Unfortunately this forces everyone calling > this > > > > method > > > > >> >> > to check the result for null. I think most code calling the > > > method > > > > >> >> > absolutely requires the BeanManager and cannot proceed > without > > > it. > > > > >> >> > > > > > >> >> > My first idea was to add another method > > getRequiredBeanManager() > > > > that > > > > >> >> > doesn't return null if the BeanManager is not available but > > > instead > > > > >> >> > throws a meaningful runtime exception. That's what Solder > does > > > per > > > > >> >> > default. Calling Solder's BeanManagerLocator.getBeanManager() > > > > without > > > > >> >> > a BeanManager being available will result in a > > > > >> >> > BeanManagerUnavailableException (see [2]). > > > > >> >> > > > > > >> >> > However Mark suggested that throwing an exception should be > the > > > > >> >> > default behavior. I for myself like Mark's idea. > > > > >> >> > > > > > >> >> > What do you think? > > > > >> >> > > > > > >> >> > Christian > > > > >> >> > > > > > >> >> > [1] https://issues.apache.org/jira/browse/DELTASPIKE-56 > > > > >> >> > [2] > > > > >> >> > > > > > >> > > > https://github.com/seam/solder/blob/master/api/src/main/java/org/jboss > > > > >> >> > /solder/beanManager/BeanManagerLocator.java#L82 > > > > >> >> > > > > > >> >> > -- > > > > >> >> > Christian Kaltepoth > > > > >> >> > Blog: http://chkal.blogspot.com/ > > > > >> >> > Twitter: http://twitter.com/chkal > > > > >> >> > > > > > >> >> > > > > >> > > > > >> > > > > >> > > > > >> -- > > > > >> Christian Kaltepoth > > > > >> Blog: http://chkal.blogspot.com/ > > > > >> Twitter: http://twitter.com/chkal > > > > >> > > > > > > > > > > > > > > > > -- > > > > Christian Kaltepoth > > > > Blog: http://chkal.blogspot.com/ > > > > Twitter: http://twitter.com/chkal > > > > > > > > > > > > > > > -- > > Jason Porter > > http://lightguard-jp.blogspot.com > > http://twitter.com/lightguardjp > > > > Software Engineer > > Open Source Advocate > > Author of Seam Catch - Next Generation Java Exception Handling > > > > PGP key id: 926CCFF5 > > PGP key available at: keyserver.net, pgp.mit.edu > > > -- Dan Allen Principal Software Engineer, Red Hat | Author of Seam in Action Registered Linux User #231597 http://google.com/profiles/dan.j.allen http://mojavelinux.com http://mojavelinux.com/seaminaction
