Update review comment related stuff

This patch addresses some review comments: 1. Updates the Javadoc. 2. Disallows arguments from being null - throws NullPointerException. 3. Updates each test to use its own DummyEventLoop. Test-information: Unit tests pass.
author: Mili Verma <mili.verma@isode.com> 2012-01-10 15:55:29 (GMT)
committer: Kevin Smith <git@kismith.co.uk> 2012-01-11 13:44:37 (GMT)
commit: be94e872d9423501c688c84c6cb945a420a5cb18 (patch)
tree: 77986e9e30282b526c0246bb6344c175b54ffb5f /src/com/isode/stroke/elements/FormField.java
parent: c9ff80a46b13b7d1987bf54cae87738a9baf552b (diff)
download: stroke-be94e872d9423501c688c84c6cb945a420a5cb18.zip
stroke-be94e872d9423501c688c84c6cb945a420a5cb18.tar.bz2
1 files changed, 180 insertions, 131 deletions
diff --git a/src/com/isode/stroke/elements/FormField.java b/src/com/isode/stroke/elements/FormField.java
index 06b2ce6..aedc8fe 100644
--- a/src/com/isode/stroke/elements/FormField.java
+++ b/src/com/isode/stroke/elements/FormField.java
@@ -17,14 +17,15 @@ import com.isode.stroke.jid.JID;
 
 /**
  * This class implements the field element of a form.
- * 
+ *
  * <p>
- * Data validation is the responsibility of the form-processing entity (commonly
- * a server, service, or bot) rather than the form-submitting entity (commonly a
- * client controlled by a human user). This helps to meet the requirement for
- * keeping client implementations simple. If the form-processing entity
- * determines that the data provided is not valid, it SHOULD return a
- * "Not Acceptable" error, optionally providing a textual explanation.
+ * From http://xmpp.org/extensions/xep-0050.html: Data validation is the
+ * responsibility of the form-processing entity (commonly a server, service, or
+ * bot) rather than the form-submitting entity (commonly a client controlled by
+ * a human user). This helps to meet the requirement for keeping client
+ * implementations simple. If the form-processing entity determines that the
+ * data provided is not valid, it SHOULD return a "Not Acceptable" error,
+ * optionally providing a textual explanation.
  */
 public class FormField {
     /**
@@ -124,33 +125,35 @@ public class FormField {
 
     /**
      * This class defines the option element that can be used in
-     * {@link ListSingleFormField} and {@link ListMultiFormField}. TODO: This
-     * class should be immutable.
+     * {@link ListSingleFormField} and {@link ListMultiFormField}. This class is
+     * immutable.
      */
     public static class Option {
         /**
-         * Human-readable name for the option, must not be null
+         * Human-readable name for the option, will not be null
          */
-        public String label;
+        public final String label;
 
         /**
-         * Option value, must not be null
+         * Option value, will not be null
          */
-        public String value;
+        public final String value;
 
         /**
          * Create an option element.
-         * 
-         * @param label Human-readable name for the option, can be null in which
-         *            case an empty string will be stored
+         *
+         * @param label Human-readable name for the option, must not be null
          * @param value Option value, must not be null
          */
         public Option(String label, String value) {
+            if (label == null) {
+                throw new NullPointerException("'label' must not be null");
+            }
             if (value == null) {
                 throw new NullPointerException("'value' must not be null");
             }
 
-            this.label = (label != null) ? label : "";
+            this.label = label;
             this.value = value;
         }
     }
@@ -168,12 +171,15 @@ public class FormField {
 
     /**
      * Set the unique identifier for the field in the form.
-     * 
-     * @param name unique identifier for the field in the form, can be null in
-     *            which case an empty string will be stored
+     *
+     * @param name unique identifier for the field in the form, must not be null
      */
     public void setName(String name) {
-        this.name = (name != null) ? name : "";
+        if (name == null) {
+            throw new NullPointerException("'name' must not be null");
+        }
+
+        this.name = name;
     }
 
     /**
@@ -185,12 +191,15 @@ public class FormField {
 
     /**
      * Set the human-readable name for the field.
-     * 
-     * @param label human-readable name for the field, can be null in which case
-     *            an empty string will be stored
+     *
+     * @param label human-readable name for the field, must not be null
      */
     public void setLabel(String label) {
-        this.label = (label != null) ? label : "";
+        if (label == null) {
+            throw new NullPointerException("'label' must not be null");
+        }
+
+        this.label = label;
     }
 
     /**
@@ -202,12 +211,16 @@ public class FormField {
 
     /**
      * Set the natural-language description for the field.
-     * 
-     * @param description natural-language description for the field, can be
-     *            null in which case an empty string will be stored
+     *
+     * @param description natural-language description for the field, must not
+     *            be null
      */
     public void setDescription(String description) {
-        this.description = (description != null) ? description : "";
+        if (description == null) {
+            throw new NullPointerException("'description' must not be null");
+        }
+
+        this.description = description;
     }
 
     /**
@@ -219,7 +232,7 @@ public class FormField {
 
     /**
      * Set if the field is required for the form to be considered valid.
-     * 
+     *
      * @param required true if the field is required for the form to be
      *            considered valid
      */
@@ -236,17 +249,20 @@ public class FormField {
 
     /**
      * Add to the list of options for this form field.
-     * 
-     * @param option Option to add, can be null in which case it will be ignored
+     *
+     * @param option Option to add, must not be null
      */
     public void addOption(Option option) {
-        if (option != null) {
-            options.add(option);
+        if (option == null) {
+            throw new NullPointerException("'option' must not be null");
         }
+
+        options.add(option);
     }
 
     /**
-     * @return List of options for this form, will never be null
+     * @return List of options for this form, will never be null. The instance
+     *         of the list stored in the object is not returned, a copy is made.
      */
     public List<Option> getOptions() {
         return new ArrayList<Option>(options);
@@ -257,15 +273,20 @@ public class FormField {
      * a form of {@link Type#FORM_TYPE} or results provided in a form of
      * {@link Type#RESULT_TYPE} or values submitted in a form of
      * {@link Type#SUBMIT_TYPE}.
-     * 
-     * @param value Value to add, can be null
+     *
+     * @param value Value to add, must not be null
      */
     public void addRawValue(String value) {
+        if (value == null) {
+            throw new NullPointerException("'value' must not be null");
+        }
+
         rawValues.add(value);
     }
 
     /**
-     * @return List of values for this field, will never be null
+     * @return List of values for this field, will never be null. The instance
+     *         of the list stored in the object is not returned, a copy is made.
      */
     public List<String> getRawValues() {
         return new ArrayList<String>(rawValues);
@@ -273,7 +294,7 @@ public class FormField {
 
     /**
      * Template for creating a form field.
-     * 
+     *
      * @param <T> Type of form field.
      */
     public static class GenericFormField<T> extends FormField {
@@ -283,10 +304,10 @@ public class FormField {
          * @return Values for this field. The values can be the defaults
          *         suggested in a form of {@link Type#FORM_TYPE} or results
          *         provided in a form of {@link Type#RESULT_TYPE} or values
-         *         submitted in a form of {@link Type#SUBMIT_TYPE}. Will never be
-         *         null.
+         *         submitted in a form of {@link Type#SUBMIT_TYPE}. Will never
+         *         be null.
          */
-        public T getValue() {
+        public final T getValue() {
             return value;
         }
 
@@ -295,10 +316,10 @@ public class FormField {
          * in a form of {@link Type#FORM_TYPE} or results provided in a form of
          * {@link Type#RESULT_TYPE} or values submitted in a form of
          * {@link Type#SUBMIT_TYPE}.
-         * 
+         *
          * @param value Value to set, must not be null
          */
-        public void setValue(T value) {
+        public void setValue(final T value) {
             if (value == null) {
                 throw new NullPointerException("'value' must not be null");
             }
@@ -306,7 +327,7 @@ public class FormField {
             this.value = value;
         }
 
-        protected GenericFormField(T value) {
+        protected GenericFormField(final T value) {
             setValue(value);
         }
     }
@@ -317,28 +338,31 @@ public class FormField {
      */
     public static class BooleanFormField extends GenericFormField<Boolean> {
         private BooleanFormField(Boolean value) {
-            super((value == null) ? Boolean.FALSE : value);
+            super(value);
         }
 
         /**
          * Create an object with given value.
-         * 
-         * @param value Value for this field, can be null which will be assumed
-         *            as FALSE.
-         * 
+         *
+         * @param value Value for this field, must not be null
+         *
          * @return new object, will never be null
          */
-        public static BooleanFormField create(Boolean value) {
+        public static BooleanFormField create(final Boolean value) {
+            if (value == null) {
+                throw new NullPointerException("'value' must not be null");
+            }
+
             return new BooleanFormField(value);
         }
 
         /**
          * Create an object with value FALSE.
-         * 
+         *
          * @return new object, will never be null
          */
         public static BooleanFormField create() {
-            return create(null);
+            return new BooleanFormField(Boolean.FALSE);
         }
     }
 
@@ -348,28 +372,31 @@ public class FormField {
      */
     public static class FixedFormField extends GenericFormField<String> {
         private FixedFormField(String value) {
-            super((value != null) ? value : "");
+            super(value);
         }
 
         /**
          * Create an object with given value.
-         * 
-         * @param value Value for this field, can be null which will be stored
-         *            as empty string.
-         * 
+         *
+         * @param value Value for this field, must not be null
+         *
          * @return new object, will never be null
          */
-        public static FixedFormField create(String value) {
+        public static FixedFormField create(final String value) {
+            if (value == null) {
+                throw new NullPointerException("'value' must not be null");
+            }
+
             return new FixedFormField(value);
         }
 
         /**
          * Create an object with value as an empty string.
-         * 
+         *
          * @return new object, will never be null
          */
         public static FixedFormField create() {
-            return create(null);
+            return new FixedFormField("");
         }
     }
 
@@ -379,28 +406,31 @@ public class FormField {
      */
     public static class HiddenFormField extends GenericFormField<String> {
         private HiddenFormField(String value) {
-            super((value != null) ? value : "");
+            super(value);
         }
 
         /**
          * Create an object with given value.
-         * 
-         * @param value Value for this field, can be null which will be stored
-         *            as empty string.
-         * 
+         *
+         * @param value Value for this field, must not be null
+         *
          * @return new object, will never be null
          */
-        public static HiddenFormField create(String value) {
+        public static HiddenFormField create(final String value) {
+            if (value == null) {
+                throw new NullPointerException("'value' must not be null");
+            }
+
             return new HiddenFormField(value);
         }
 
         /**
          * Create an object with value as an empty string.
-         * 
+         *
          * @return new object, will never be null
          */
         public static HiddenFormField create() {
-            return create(null);
+            return new HiddenFormField("");
         }
     }
 
@@ -410,28 +440,31 @@ public class FormField {
      */
     public static class ListSingleFormField extends GenericFormField<String> {
         private ListSingleFormField(String value) {
-            super((value != null) ? value : "");
+            super(value);
         }
 
         /**
          * Create an object with given value.
-         * 
-         * @param value Value for this field, can be null which will be stored
-         *            as empty string.
-         * 
+         *
+         * @param value Value for this field, must not be null
+         *
          * @return new object, will never be null
          */
-        public static ListSingleFormField create(String value) {
+        public static ListSingleFormField create(final String value) {
+            if (value == null) {
+                throw new NullPointerException("'value' must not be null");
+            }
+
             return new ListSingleFormField(value);
         }
 
         /**
          * Create an object with value as an empty string.
-         * 
+         *
          * @return new object, will never be null
          */
         public static ListSingleFormField create() {
-            return create(null);
+            return new ListSingleFormField("");
         }
     }
 
@@ -441,28 +474,31 @@ public class FormField {
      */
     public static class TextMultiFormField extends GenericFormField<String> {
         private TextMultiFormField(String value) {
-            super((value != null) ? value : "");
+            super(value);
         }
 
         /**
          * Create an object with given value.
-         * 
-         * @param value Value for this field, can be null which will be stored
-         *            as empty string.
-         * 
+         *
+         * @param value Value for this field, must not be null
+         *
          * @return new object, will never be null
          */
-        public static TextMultiFormField create(String value) {
+        public static TextMultiFormField create(final String value) {
+            if (value == null) {
+                throw new NullPointerException("'value' must not be null");
+            }
+
             return new TextMultiFormField(value);
         }
 
         /**
          * Create an object with value as an empty string.
-         * 
+         *
          * @return new object, will never be null
          */
         public static TextMultiFormField create() {
-            return create(null);
+            return new TextMultiFormField("");
         }
     }
 
@@ -473,28 +509,31 @@ public class FormField {
      */
     public static class TextPrivateFormField extends GenericFormField<String> {
         private TextPrivateFormField(String value) {
-            super((value != null) ? value : "");
+            super(value);
         }
 
         /**
          * Create an object with given value.
-         * 
-         * @param value Value for this field, can be null which will be stored
-         *            as empty string.
-         * 
+         *
+         * @param value Value for this field, must not be null
+         *
          * @return new object, will never be null
          */
-        public static TextPrivateFormField create(String value) {
+        public static TextPrivateFormField create(final String value) {
+            if (value == null) {
+                throw new NullPointerException("'value' must not be null");
+            }
+
             return new TextPrivateFormField(value);
         }
 
         /**
          * Create an object with value as an empty string.
-         * 
+         *
          * @return new object, will never be null
          */
         public static TextPrivateFormField create() {
-            return create(null);
+            return new TextPrivateFormField("");
         }
     }
 
@@ -504,28 +543,31 @@ public class FormField {
      */
     public static class TextSingleFormField extends GenericFormField<String> {
         private TextSingleFormField(String value) {
-            super((value != null) ? value : "");
+            super(value);
         }
 
         /**
          * Create an object with given value.
-         * 
-         * @param value Value for this field, can be null which will be stored
-         *            as empty string.
-         * 
+         *
+         * @param value Value for this field, must not be null
+         *
          * @return new object, will never be null
          */
-        public static TextSingleFormField create(String value) {
+        public static TextSingleFormField create(final String value) {
+            if (value == null) {
+                throw new NullPointerException("'value' must not be null");
+            }
+
             return new TextSingleFormField(value);
         }
 
         /**
          * Create an object with value as an empty string.
-         * 
+         *
          * @return new object, will never be null
          */
         public static TextSingleFormField create() {
-            return create(null);
+            return new TextSingleFormField("");
         }
     }
 
@@ -534,28 +576,31 @@ public class FormField {
      */
     public static class JIDSingleFormField extends GenericFormField<JID> {
         private JIDSingleFormField(JID value) {
-            super((value != null) ? value : new JID());
+            super(value);
         }
 
         /**
          * Create an object with given value.
-         * 
-         * @param value Value for this field, can be null which will be stored
-         *            as an invalid JID.
-         * 
+         *
+         * @param value Value for this field, must not be null
+         *
          * @return new object, will never be null
          */
-        public static JIDSingleFormField create(JID value) {
+        public static JIDSingleFormField create(final JID value) {
+            if (value == null) {
+                throw new NullPointerException("'value' must not be null");
+            }
+
             return new JIDSingleFormField(value);
         }
 
         /**
          * Create an object with value as an invalid JID.
-         * 
+         *
          * @return new object, will never be null
          */
         public static JIDSingleFormField create() {
-            return create(null);
+            return new JIDSingleFormField(new JID());
         }
     }
 
@@ -564,30 +609,32 @@ public class FormField {
      */
     public static class JIDMultiFormField extends GenericFormField<List<JID>> {
         private JIDMultiFormField(List<JID> value) {
-            super((value == null) ? new ArrayList<JID>() : new ArrayList<JID>(
-                    value));
+            super(value);
         }
 
         /**
          * Create an object with given value.
-         * 
-         * @param value Value for this field, can be null which will be assumed
-         *            to be an empty list. A copy of the given list will be kept
-         *            by this object.
-         * 
+         *
+         * @param value Value for this field, must not be null. A copy of the
+         *            given list will be kept by this object.
+         *
          * @return new object, will never be null
          */
-        public static JIDMultiFormField create(List<JID> value) {
-            return new JIDMultiFormField(value);
+        public static JIDMultiFormField create(final List<JID> value) {
+            if (value == null) {
+                throw new NullPointerException("'value' must not be null");
+            }
+
+            return new JIDMultiFormField(new ArrayList<JID>(value));
         }
 
         /**
          * Create an object with an empty list.
-         * 
+         *
          * @return new object, will never be null
          */
         public static JIDMultiFormField create() {
-            return create(null);
+            return new JIDMultiFormField(new ArrayList<JID>());
         }
     }
 
@@ -598,30 +645,32 @@ public class FormField {
     public static class ListMultiFormField extends
     GenericFormField<List<String>> {
         private ListMultiFormField(List<String> value) {
-            super((value == null) ? new ArrayList<String>()
-                    : new ArrayList<String>(value));
+            super(value);
         }
 
         /**
          * Create an object with given value.
-         * 
-         * @param value Value for this field, can be null which will be assumed
-         *            to be an empty list. A copy of the given list will be kept
-         *            by this object.
-         * 
+         *
+         * @param value Value for this field, must not be null. A copy of the
+         *            given list will be kept by this object.
+         *
          * @return new object, will never be null
          */
-        public static ListMultiFormField create(List<String> value) {
-            return new ListMultiFormField(value);
+        public static ListMultiFormField create(final List<String> value) {
+            if (value == null) {
+                throw new NullPointerException("'value' must not be null");
+            }
+
+            return new ListMultiFormField(new ArrayList<String>(value));
         }
 
         /**
          * Create an object with an empty list.
-         * 
+         *
          * @return new object, will never be null
          */
         public static ListMultiFormField create() {
-            return create(null);
+            return new ListMultiFormField(new ArrayList<String>());
         }
     }
author	Mili Verma <mili.verma@isode.com>	2012-01-10 15:55:29 (GMT)
committer	Kevin Smith <git@kismith.co.uk>	2012-01-11 13:44:37 (GMT)
commit	be94e872d9423501c688c84c6cb945a420a5cb18 (patch)
tree	77986e9e30282b526c0246bb6344c175b54ffb5f /src/com/isode/stroke/elements/FormField.java
parent	c9ff80a46b13b7d1987bf54cae87738a9baf552b (diff)
download	stroke-be94e872d9423501c688c84c6cb945a420a5cb18.zip stroke-be94e872d9423501c688c84c6cb945a420a5cb18.tar.bz2