+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
