Validation of instance variables

[Kev Jackson]

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
Index: Task.java
===================================================================
RCS file: /home/cvspublic/ant/src/main/org/apache/tools/ant/Task.java,v
retrieving revision 1.64
diff -u -r1.64 Task.java
--- Task.java   22 Mar 2005 16:39:33 -0000      1.64
+++ Task.java   25 Mar 2005 08:56:26 -0000
@@ -19,8 +19,10 @@
 
 import org.apache.tools.ant.dispatch.DispatchUtils;
 
+import java.util.Collection;
 import java.util.Enumeration;
 import java.io.IOException;
+import java.lang.reflect.Field;
 
 /**
  * Base class for all tasks.
@@ -490,4 +492,13 @@
         setLocation(owner.getLocation());
         setTaskType(owner.getTaskType());
     }
+    
+    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 != null && 
!optional.contains(fields[i].getName())) {
+                throw new BuildException(""+fields[i].getName()+" attribute 
required", getLocation());
+            }
+        }
+    }
 }
Index: Mkdir.java
===================================================================
RCS file: 
/home/cvspublic/ant/src/main/org/apache/tools/ant/taskdefs/Mkdir.java,v
retrieving revision 1.29
diff -u -r1.29 Mkdir.java
--- Mkdir.java  9 Mar 2004 16:48:06 -0000       1.29
+++ Mkdir.java  25 Mar 2005 08:57:20 -0000
@@ -18,6 +18,8 @@
 package org.apache.tools.ant.taskdefs;
 
 import java.io.File;
+import java.util.Vector;
+
 import org.apache.tools.ant.BuildException;
 import org.apache.tools.ant.Task;
 
@@ -44,10 +46,7 @@
      * @throws BuildException if dir is somehow invalid, or creation failed.
      */
     public void execute() throws BuildException {
-        if (dir == null) {
-            throw new BuildException("dir attribute is required", 
getLocation());
-        }
-
+        validate(this.getClass(), new Vector(0));
         if (dir.isFile()) {
             throw new BuildException("Unable to create directory as a file "
                                      + "already exists with that name: "
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]