ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan.Mate...@rzf.fin-nrw.de
Subject AW: Validation of instance variables
Date Tue, 29 Mar 2005 06:48:06 GMT
Nice idea, but I think that´s a little overkill for just checking against
null.
Just checking the javadoc [1]: inherited fields are not catched by that
method. Without
having checked that, I could think that the AbstractCvsTask (I think there
is one...)
could contain some "must-not-null-fields". Same could be for the Continuus
tasks.

But if we introduce that, I wouldnt name that "validate" because it "just"
checks against
null. So something like 
   assertNotNull(Class c)
   assertNotNull(Class c, Collection optionalFields)
would be more meaningful.

That check doesnt seem to need any project relevant data - just the class
itself which is
provided as argument. We could provide that as a util class
"oata.util.ClassUtils" (Singleton
as FileUtils et al).


Jan


[1]
http://java.sun.com/j2se/1.4.2/docs/api/java/lang/Class.html#getDeclaredFiel
ds()

> -----Ursprüngliche Nachricht-----
> Von: Kev Jackson [mailto:kevin.jackson@it.fts-vn.com]
> Gesendet am: Freitag, 25. März 2005 10:01
> An: Ant Developers List
> Betreff: Validation of instance variables
> 
> We usually have to check to ensure that various values aren't null 
> before calling execute() in the tasks.
> 
> I've been thinking about this, and it seems that at the moment every 
> task handles it differently.  Some check the values as part of 
> theexecute method, some have a seperate private method that 
> gets called 
> before execute.  I'd like to unify this checking, just out of 
> cleanliness, so how about something like...
> 
> public void validate(Class c) throws BuildException  {
>         Field[] fields = c.getDeclaredFields();
>         for (int i=0; i<fields.length; i++) {
>             if (fields[i] == null) {
>                 throw new BuildException(""+fields[i].getName()+" 
> attribute required");
>             }
>         }
>     }
> 
> would work fine, if all attributes were required, but some are only 
> optional, so
> 
> public void validate(Class c, Collection optional) throws 
> BuildException  {
>         Field[] fields = c.getDeclaredFields();
>         for (int i=0; i<fields.length; i++) {
>             if (fields[i] == null && 
> !optional.contains(fields[i].getName())) {
>                 throw new BuildException(""+fields[i].getName()+" 
> attribute required");
>             }
>         }
>     }
> may be more appropriate.  Then you could have this as a method in the 
> Task and simply call
> 
> super.validate(this, new ArrayList());
> if all fields are required, or
> 
> super.validate(this, new ArrayList("optionalProp1", "optionalProp2"));
> if all fields except optionalProp1 and optionalProp2 are required
> 
> This would move the vaildation up into the Task (where I 
> think not-null 
> checking makes more sense) and it makes it simple for tasks 
> to override 
> if more complex ("this one can only be null if this one is 
> set") kind of 
> checks are required.
> 
> I've attached the patched Task and a (trivial) example.
> 
> Thoughts?  Comments?
> 
> Kev
> 

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message