Re: [Figleaf-developer] Validation and PropertyDescriptor

[<sam...@ma...>]

Quoting Greg Steckman <ste...@on...>:


> >1. It exposes an implementation detail - what if I want my property's setter
> to
> >call several methods for example?
> >
> You could write a setter method (call it methodA) that calls the several 
> methods. Then specify methodA as the property write method.

You could, but you don't have to if you hide what is actually being done.

> >2. It completely bypasses all validation logic that might apply to a
> property -
> >you would have to hope that any UI layer validates before calling read or
> write,
> >resulting in the developer not being able to guarantee that their
> (business)
> >validation logic would ever be invoked.
> >
> 
> Yes, this is why I preferred the other approach of having validation 
> occur automatically. It would either be implemented directly by the 
> writer of the Informative, or we could do some type of proxying to 
> implement it.

Which you cannot enforce if you expose a Method without some complex proxying.
What benifit does exposing the Method have over having the PropertDescriptor
execute the method for us?
> 
> >3. Calling a property becomes more complex than needed
> >
> >I prefer the following solution - we add the following method to
> PropertyDescriptor:
> >
> >  /**
> >   * Invokes validation without setting the value - useful in UI's to
> provide 
> >   * immediate feedback to the user of invalid values etc.
> >   */
> >  boolean isValid(Object newValue, ValidationCallback callback);
> >
> >Next, get rid of getReadMethod and getWriteMethod and instead use:
> >
> >  Object read();
> >
> >  /**
> >   * Sets a new value if valid, else callback.invalid() will be called
> >   */
> >  void write(Object newValue, ValidationCallback callback);
> >
> >For this PropertyDescriptor would have to become a per-instance object
> (which I
> >don't think is a big problem). If we wanted to keep PropertyDescriptor as
> >per-class, then we'd need to pass in an additional parameter into
> read/write.
> >Also note that there is now no need to expose ValidationRules outside of
> the
> >PropertyDescriptor, helping keep the responsibility for determining an
> Objects
> >validity inside the Object itself.


> Let's consider an implementation of a class that implements the 
> Informative interface itself.
> 
> Case A: Current implementation +  modify PropertyDescriptor to add the 
> methods:
> void addValidationListener(ValidationListener listener);
> void removeValidationListener(ValidationListener listener);

What would listen to validation events? The GUI - in fact parts of the GUI would
only need to listen to specific properties. As such I think a more fine-grained
validation listener approach would benifit us - allow validation callbacks on
specific property calls. By passing a callback into a validation call (or even
as a result of calling get) the Informative object doesn't need to manage
multiple ValidationListeners, and the GUI doesn't have to worry about detaching
listeners

> public class MyInformative implements Informative{
>     PropertyDescriptor[] pds;
>     Object propA;
> 
>     public void setPropertyA(Object value){
>        boolean invalid=true;
> 
>        //do some validation logic
>        //set invalid as required

>        if(invalid){
>           pds[0].fireValidationInvalid("reason string goes here");
>        }else{
>           propA=value;         
>           pds[0].fireValidationValid();
>        }
>     }


You are now poluting property accesing methods with figleaf dependant code.
Developers now need to explicitly invoke validation for all their set methods,
and have to do so in a way that ties their object to figleaf. I prefer to deal
with validation at the point we describe the properties (or general 'action'
methods) to the figleaf system, that is in the PropertyDescriptor.

>     public Object getPropertyA(){return propA;}
> 
>     PropertyDescriptor[] getPropertyDesctiptors(){
>        pds=new PropertyDescriptor[1];
>        pds[0]=new PropertyDescriptorImpl(Object.class, getClass(),
>                                                                  
> getMethod("getPropertyA", null).
>                                                                  
> getMethod("setPropertyA", new Class[]{Object.class}),
>                                                                   
> "Property A",
>                                                                    
> "Description of Property A");
>        return pds;
>     }
> }
> 
> 
> Case B: The implementation you suggest:
> 
> public class MyInformative implements Informative{
>     public void setPropertyA(Object value){...}
>     public Object getPropertyA(){...}
> 
>     PropertyDescriptor[] getPropertyDesctiptors(){
>        PropertyDescriptor[] pds=new PropertyDescriptor[1];
>        pds[0]=new MyInformativePropertyDescriptor(
>                                                                  
> getMethod("getPropertyA", null).
>                                                                  
> getMethod("setPropertyA", new Class[]{Object.class}),
>                                                                   
> "Property A",
>                                                                    
> "Description of Property A");
>        return pds;
>     }
> 
>     class MyInformativePropertyDescriptor implements PropertyDescriptor{
>            private Method readMethod, writeMethod;
>            private String name, description;
> 
>           MyInformativePropertyDescriptor(Method readMethod, Method 
> writeMethod, String name, String desc){
>              this.readMethod=readMethod;
>              this.writeMethod=writeMethod;
>              this.name=name;
>              this.description=desc;
>           }
> 
>           public Object read(){
>                 return readMethod.invoke(this.MyInformative, null);
>           }
> 
>         public void write(Object o, ValidationCallback callback){
>             //do validation
> 
>             if(valid){
>                 writeMethod.invoke(this.MyInformative, new Object[]{o});
>             }      
>        }
>      }
> }

I wouldn't call methods like that - I'd call them directly on the object to gain
compile-time checking and simpler looking code.
 
> In the first case I used an object that's defined elsewhere so that 
> makes the code look shorter but in reality it probably won't be. I 
> prefer the listener approach of Case A because I think it has more 
> flexibility and is a standard pattern so users will understand the API 
> right away.

But users have to define validation logic and invoke it themselves - my approach
just has them define validation rules, and everything else is handled for them,
making thw system simpler for them to use.

> If we do that, I don't see the benefit of not using Methods 
> in the PropertyDescriptor.

You then require that figleaf has to proxy method calls. You also expose
implementation detail. The UI then has to invoke the method itself, catch the
exceptions etc. What about remoting? What if invoking the property actually
invokes a webservices call? Now we have to create a fake Method which we can't
easily do (Method is a final class).

> I also prefer using the Methods because I 
> think they are more flexible. They can reference any method in any class 
> without having to implement an extra class just for redirection.

I don't agree really - where would you need an object to expose the method of
another class?

> If a class doesn't implement Informative itself, then everything needs 
> to be handled by the Proxy in either case.
> 
> Greg

Which I already do - exposing the method is no simpler than directly invoking it
inside the property, I've had proxies working in both ways.

In reality validation at a property-by-property basis is simple from developer
terms, but is often not good enough, for example what if a customer bean has 3
telephone number fields, and one of them has to be non-null? Neither of our
approaches work, as we are still validating on a property-by-property basis -
even a final-mile validation check on the object as a whole happens to late, as
by then the object might be in an invalid state.

I say lets rethink the problem. We need to not only validate a single property
change, but the validaity of a set of changes on an object. Lets add the
following method to Informative:

boolean commitChanges(ChangeSet set)

Lets keep your add and remove validation listeners on Informative. A ChangeSet
is a set of changes on an object, and now becomes the only mechanism by which
figleaf writes changes to an object - it consists of a series of property change
messages. I'll knock up a prototype in the next day or so and see if its a
workable idea.

sam
http://www.magpiebrain.com/