From 265e5c69d501e32bbe78f245fae08e9fdec931cb Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Sun, 12 Apr 2015 19:27:22 +0200 Subject: [PATCH] Use LinkedHashMap for form fields for efficient lookup. --- .../filetransfer/FileTransferNegotiator.java | 7 +---- .../org/jivesoftware/smackx/xdata/Form.java | 11 +------ .../jivesoftware/smackx/xdata/FormField.java | 5 ++-- .../smackx/xdata/packet/DataForm.java | 30 ++++++++++++------- 4 files changed, 25 insertions(+), 28 deletions(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java index 726ca6be0..e978707b7 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java @@ -215,12 +215,7 @@ public final class FileTransferNegotiator extends Manager { } private static FormField getStreamMethodField(DataForm form) { - for (FormField field : form.getFields()) { - if (field.getVariable().equals(STREAM_DATA_FIELD_NAME)) { - return field; - } - } - return null; + return form.getField(STREAM_DATA_FIELD_NAME); } private StreamNegotiator getNegotiator(final FormField field) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/Form.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/Form.java index 7d376e103..58d8459d7 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/Form.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/Form.java @@ -350,16 +350,7 @@ public class Form { * @return the field of the form whose variable matches the specified variable. */ public FormField getField(String variable) { - if (variable == null || variable.equals("")) { - throw new IllegalArgumentException("Variable must not be null or blank."); - } - // Look for the field whose variable matches the requested variable - for (FormField field : getFields()) { - if (variable.equals(field.getVariable())) { - return field; - } - } - return null; + return dataForm.getField(variable); } /** diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/FormField.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/FormField.java index 570a9ed8d..11c6a939e 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/FormField.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/FormField.java @@ -22,6 +22,7 @@ import java.util.Collections; import java.util.List; import org.jivesoftware.smack.packet.NamedElement; +import org.jivesoftware.smack.util.StringUtils; import org.jivesoftware.smack.util.XmlStringBuilder; import org.jivesoftware.smackx.xdatavalidation.packet.ValidateElement; @@ -150,7 +151,7 @@ public class FormField implements NamedElement { * @param variable the variable name of the question. */ public FormField(String variable) { - this.variable = variable; + this.variable = StringUtils.requireNotNullOrEmpty(variable, "Variable must not be null or empty"); } /** @@ -158,7 +159,7 @@ public class FormField implements NamedElement { * name. */ public FormField() { - this(null); + this.variable = null; this.type = Type.fixed; } diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/packet/DataForm.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/packet/DataForm.java index 4ccb9d85f..577c06831 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/packet/DataForm.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/xdata/packet/DataForm.java @@ -25,8 +25,10 @@ import org.jivesoftware.smackx.xdata.FormField; import java.util.ArrayList; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; +import java.util.Map; /** * Represents a form that could be use for gathering data as well as for reporting data @@ -71,7 +73,7 @@ public class DataForm implements ExtensionElement { private List instructions = new ArrayList(); private ReportedData reportedData; private final List items = new ArrayList(); - private final List fields = new ArrayList(); + private final Map fields = new LinkedHashMap<>(); private final List extensionElements = new ArrayList(); public DataForm(Type type) { @@ -139,7 +141,7 @@ public class DataForm implements ExtensionElement { */ public List getFields() { synchronized (fields) { - return Collections.unmodifiableList(new ArrayList(fields)); + return new ArrayList<>(fields.values()); } } @@ -152,13 +154,21 @@ public class DataForm implements ExtensionElement { */ public FormField getField(String variableName) { synchronized (fields) { - for (FormField field : fields) { - if (variableName.equals(field.getVariable())) { - return field; - } - } + return fields.get(variableName); + } + } + + /** + * Check if a form field with the given variable name exists. + * + * @param variableName + * @return true if a form field with the variable name exists, false otherwise. + * @since 4.2 + */ + public boolean hasField(String variableName) { + synchronized (fields) { + return fields.containsKey(variableName); } - return null; } public String getElementName() { @@ -206,12 +216,12 @@ public class DataForm implements ExtensionElement { */ public void addField(FormField field) { String fieldVariableName = field.getVariable(); - if (fieldVariableName != null && getField(fieldVariableName) != null) { + if (hasField(fieldVariableName)) { throw new IllegalArgumentException("This data form already contains a form field with the variable name '" + fieldVariableName + "'"); } synchronized (fields) { - fields.add(field); + fields.put(fieldVariableName, field); } }