Yep, I agree we're in subjective land here. We could add a method to XhtmlRenderer that does the same "get-if-not-null", which would avoid adding a bonus method to a core public class.
-- Adam On 8/29/07, Simon Lessard <[EMAIL PROTECTED]> wrote: > I agree with Adam about the logging issue. The point of the change would be > to make it ok to pass null key to the FacesBean so that renderer can save a > redundant 'if' to check if the property is supported on the current > component. So INFO level would spam the log for nothing. > > This suggestion is completely subjective. We cannot even base ourselves on > java.util.Map semantics to keep a consistent behavior for users as some Map > implementations permits null keys while other don't. I may have another > option that might resolve it all. We could have two methods in FacesBeans, > .getProperty and something like getPropertyIfSupported. The former would > works just like now while the latter would first check for null key. That > way, renderers able to work properly even if the property does not exist > would use getPropertyIfSupported in their getPropertyName(FacesBean) > protected method, and the .getProperty one if the property is mandatory to > ensure correct behavior. This last solution would not break existing code in > any way and would give an utility method to reduce boilerplate code within > renderer code. > > > Regards, > > ~ Simon > > > On 8/28/07, Adam Winer <[EMAIL PROTECTED]> wrote: > > Logging as INFO seems the worst of all worlds. > > Either null property keys are right or wrong. > > > > If they're wrong, then logging at SEVERE and returning null might be > > OK, NullPointerException is also acceptable - which you want depends > > mostly on who will see the error. If end users will see the error, > logging at > > SEVERE is (often) better. If developers on the project will see it, > throwing > > an exception is correct. > > > > If null PropertyKeys are OK, then logging at FINE or FINER or FINEST > > is acceptable - but not at INFO. > > > > -- Adam > > > > > > > > On 8/28/07, Steven Murray <[EMAIL PROTECTED]> wrote: > > > This type of thing should be logged but not throw an exception. The > level of error should be INFO. So I like the idea of returning null, > certainly null pointer is awful but throwing an exception is not much of an > improved either. > > > > > > -- Steve > > > > > > ________________________________ > > > > > > From: Adam Winer [mailto:[EMAIL PROTECTED] > > > Sent: Tue 8/28/2007 4:52 PM > > > To: MyFaces Development > > > Subject: Re: [Trinidad] Should FacesBean.getProperty(null) really throw > an exception? > > > > > > > > > > > > I'd personally rather have it throw an exception - it makes it harder to > > > catch errors if nulls are ignored in general. > > > > > > -- Adam > > > > > > > > > On 8/28/07, Simon Lessard <[EMAIL PROTECTED]> wrote: > > > > Hello all, > > > > > > > > Currenty, FacesBean.getProperty(null) throws a NullPointerException > from the > > > > checkNotListKey method call. However, I feel it should rather return > null > > > > and not throw an exception. That way, we would no longer have to use > the > > > > following code snippet in our renderer to know if the property is > supported > > > > on any given component: > > > > if (someKey == null) > > > > { > > > > return null; > > > > } > > > > else > > > > { > > > > return ComponentUtils.resolveSomeType(bean.getProperty(someKey), > > > > someDefaultValue); > > > > } > > > > > > > > If getProperty were to return null instead of throwing an exception, > only > > > > return line would be needed, reducing some boilerplate code in the > various > > > > renderers. > > > > > > > > Any objection to make that change to FacesBeanImpl and reflect it in > the > > > > FacesBean's javadoc? > > > > > > > > > > > > Regards, > > > > > > > > ~ Simon > > > > > > > > > > > > > > > > > > > >
