Adding it to XhtmlRenderer sounds like a good compromise. So what name should be used? Since it's going to return the default value as well, maybe something along the lines of protected Object resolveProperty(FacesBean bean, PropertyKey key);.?
Regards, ~ Simon On 8/29/07, Adam Winer <[EMAIL PROTECTED]> wrote: > > Yeah, good point. > > Background - when I designed FacesBean way back when, > I very intentionally made sure that FacesBean did not automatically > return default values, because I wanted the core API to not lose the > information of "has this value been set?" vs. "has this value been > explicitly set to the default?". The UIComponent APIs wrap that up, > but Renderers directly accessing the FacesBean then have to > remember to check the default. So there's a lot of boilerplate renderer > code that checks PropertyKey default values. Which like all > boilerplate code is lame. > > -- Adam > > > > On 8/29/07, Danny Robinson <[EMAIL PROTECTED]> wrote: > > > > Can I request this api also optionally returns the default for an > > attribute? > > > > On 8/29/07, Adam Winer < [EMAIL PROTECTED] > wrote: > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > Chordiant Software Inc. > > www.chordiant.com > > >
