Re: [Figleaf-developer] Validation and PropertyDescriptor

[Greg S. <ste...@on...>]

sam...@ma... wrote:

>In the light of my work on Validation, I think we need to re-evaluate the
>PropertyDescriptor, at least the way in which it exposes the read and write methods.
>
>If we take the view that Validation is used to verify whether or not a value is
>valid for a domain object, then it is fair to say the validation is business
>logic. Currently, to invoke a property, you call one of these two methods on
>PropertyDescriptor:
>
> Method getReadMethod();
>
> Method getWriteMethod(); 
>
>I dislike this for a variety of reasons:
>
>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.

>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.

>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.
>
>sam
>
>
>  
>

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);

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();
       }
    }

    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});
            }      
       }
     }
}


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. If we do that, I don't see the benefit of not using Methods 
in the PropertyDescriptor. 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.

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

Greg