dev@javaserverfaces.java.net

Re: Seeking Review - JSFRI 165

From: Ed Burns <ed.burns_at_sun.com>
Date: Tue, 9 Aug 2005 09:28:37 -0700

>>>>> On Tue, 09 Aug 2005 11:21:10 -0400, Roger Kitain <Roger.Kitain_at_Sun.COM> said:

RK> Please review:
RK> https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=165

RK>
RK> << ADD DESCRIPTION HERE >>
RK>
RK>
RK> SECTION: Modified Files
RK> ----------------------------
RK> M jsf-ri/build-tests.xml
RK> --- added new test case
RK> M jsf-ri/src/com/sun/faces/application/StateManagerImpl.java
RK> --- implemented non-deprecated methods [saveView, writeState(FacesContext, Object)]

RK> + public Object saveView(FacesContext context) {
RK> + return ( super.saveView(context) );
RK> + }
RK> +

RK> - private boolean hasDeclaredMethod(Map resultMap, Object key, Object instance, String methodName) {
RK> + public boolean hasDeclaredMethod(Map resultMap, Object key, Object instance, String methodName) {

I don't like how we have two methods with the same name, one on
StateManagerImpl, one on ViewHandlerImpl. Can't these be refactored
into a Util method, with parameters to control operation? Of course,
that refactoring can sometimes lead to more confusion, but I'd at least
want to see it considered.

Can you please answer this concern, perhaps with another change-bundle?

RK> M jsf-ri/src/com/sun/faces/application/ViewHandlerImpl.java
RK> --- introduced method to check for existence of non-deprecated method
RK> on StateManager implementation instance.
RK> --- perform checks for existence of a single non-deprecated method (saveView).
RK> If this method exists on the implementation, then the assumption is that
RK> the other non-deprecated method (writeState(FacesContextm, Object) will
RK> also be implemented. Call the non-deprecated methods if they exist; otherwise
RK> fall back to calling the deprecated ones.

I don't think you need a map as you did in the case of RSM because there
may be only one StateManager per application. It's not dependent on
RenderKit, and cannot switch dynamically as you run the app. Dealing
with this is probably the biggest obstacle to having one
hasDeclaredMethod() in Util. There are ways of overcoming this
obstacle, however.

Ed

-- 
| ed.burns_at_sun.com  | {home: 407 869 9587, office: 408 884 9519 OR x31640}
| homepage:         | http://purl.oclc.org/NET/edburns/
| aim: edburns0sunw | iim: ed.burns_at_sun.com