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:[EMAIL PROTECTED] > 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 >
