I don't see a need for adding the default flag at all... Danny,what's your thinking behind this?
-- Adam On 8/29/07, Simon Lessard <[EMAIL PROTECTED]> wrote: > > I don't mind the flag, but then I would add both methods with one using > 'true' as default value for the default flag. > > > On 8/29/07, Danny Robinson <[EMAIL PROTECTED]> wrote: > > > > Maybe > > > > protected Object resolveProperty(FacesBean bean, PropertyKey key, > > boolean default);. > > > > On 8/29/07, Simon Lessard <[EMAIL PROTECTED]> wrote: > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > -- > > Chordiant Software Inc. > > www.chordiant.com > > > >
