jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1513811 - in /jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http: control/gui/HttpTestSampleGui.java sampler/HTTPSamplerBase.java
Date Wed, 14 Aug 2013 12:00:11 GMT
This is simpler, but I still think there are some unnecessary
conversions between index and name.
It seems to me that the dropdown index can be used directly as the JMX value.

Only the combo box actually needs to know the string values, and the
GUI code could do the translation from property to string.

The order of the strings is critical, but this is not currently
guaranteed as there is no relationship between the SOURCE_TYPE_*
constants and the order of adding the strings to the ArrayList. (That
was one way in which the old code was better).

We could use set(index, String) instead of add(String); that would
guarantee tie the index to the String and guarantee the correct
ordering.

However, from the point of view of using the selected source type in
code, it would be better to use an enum.
If the enum stored the property name, this could be used by the GUI to
derive the text.

The enum entry could be derived from the index using SourceType.values()[index]

Any objections if I make the changes suggested above?
[Probably easier for me to do this and then others can criticise my code.]

On 14 August 2013 12:08,  <milamber@apache.org> wrote:
> Author: milamber
> Date: Wed Aug 14 11:08:59 2013
> New Revision: 1513811
>
> URL: http://svn.apache.org/r1513811
> Log:
> Change Source Type Map to ArrayList (better)
> Bugzilla Id: 54874
>
> Modified:
>     jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java
>     jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
>
> Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java?rev=1513811&r1=1513810&r2=1513811&view=diff
> ==============================================================================
> --- jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java
(original)
> +++ jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/gui/HttpTestSampleGui.java
Wed Aug 14 11:08:59 2013
> @@ -72,7 +72,7 @@ public class HttpTestSampleGui extends A
>
>      private JTextField sourceIpAddr; // does not apply to Java implementation
>
> -    private JComboBox sourceIpType = new JComboBox(HTTPSamplerBase.getSourceTypeMap().keySet().toArray());
> +    private JComboBox sourceIpType = new JComboBox(HTTPSamplerBase.getSourceTypeList().toArray());
>
>      private final boolean isAJP;
>
> @@ -136,7 +136,7 @@ public class HttpTestSampleGui extends A
>          samplerBase.setEmbeddedUrlRE(embeddedRE.getText());
>          if (!isAJP) {
>              samplerBase.setIpSource(sourceIpAddr.getText());
> -            samplerBase.setIpSourceType(HTTPSamplerBase.getSourceTypeMap().get(sourceIpType.getSelectedItem()).intValue());
> +            samplerBase.setIpSourceType(HTTPSamplerBase.getSourceTypeList().indexOf(sourceIpType.getSelectedItem()));
>          }
>          this.configureTestElement(sampler);
>      }
> @@ -241,7 +241,8 @@ public class HttpTestSampleGui extends A
>
>          if (!isAJP) {
>              // Add a new field source ip address (for HC implementations only)
> -            sourceIpType.setSelectedItem(JMeterUtils.getResString("web_testing_source_ip_hostname"));
 //$NON-NLS-1$ default: IP/Hostname
> +            sourceIpType.setSelectedItem(HTTPSamplerBase.getSourceTypeList().
> +                    get(HTTPSamplerBase.SOURCE_TYPE_IP_HOSTNAME));  //default: IP/Hostname
>              sourceIpType.setFont(FONT_VERY_SMALL);
>              sourceAddrPanel.add(sourceIpType);
>
> @@ -275,7 +276,8 @@ public class HttpTestSampleGui extends A
>          embeddedRE.setText(""); // $NON-NLS-1$
>          if (!isAJP) {
>              sourceIpAddr.setText(""); // $NON-NLS-1$
> -            sourceIpType.setSelectedItem(JMeterUtils.getResString("web_testing_source_ip_hostname"));
 //$NON-NLS-1$
> +            sourceIpType.setSelectedItem(HTTPSamplerBase.getSourceTypeList()
> +                    .get(HTTPSamplerBase.SOURCE_TYPE_IP_HOSTNAME));
>          }
>      }
>
>
> Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java?rev=1513811&r1=1513810&r2=1513811&view=diff
> ==============================================================================
> --- jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
(original)
> +++ jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
Wed Aug 14 11:08:59 2013
> @@ -31,7 +31,6 @@ import java.util.Collections;
>  import java.util.HashMap;
>  import java.util.HashSet;
>  import java.util.Iterator;
> -import java.util.LinkedHashMap;
>  import java.util.List;
>  import java.util.Map;
>  import java.util.Set;
> @@ -198,14 +197,14 @@ public abstract class HTTPSamplerBase ex
>
>      public static final int SOURCE_TYPE_DEVICE_IPV6 = 3;
>
> -    // Use for ComboBox Source Address Type. LinkedHashMap to preserve order (specially
with localization)
> -    public static final LinkedHashMap<String, Integer> getSourceTypeMap() {
> -        LinkedHashMap<String, Integer> sourceTypeMap = new LinkedHashMap<String,
Integer>(4);
> -        sourceTypeMap.put(JMeterUtils.getResString("web_testing_source_ip_hostname"),
SOURCE_TYPE_IP_HOSTNAME); //$NON-NLS-1$
> -        sourceTypeMap.put(JMeterUtils.getResString("web_testing_source_ip_device"),
SOURCE_TYPE_DEVICE); //$NON-NLS-1$
> -        sourceTypeMap.put(JMeterUtils.getResString("web_testing_source_ip_device_ipv4"),
SOURCE_TYPE_DEVICE_IPV4); //$NON-NLS-1$
> -        sourceTypeMap.put(JMeterUtils.getResString("web_testing_source_ip_device_ipv6"),
SOURCE_TYPE_DEVICE_IPV6); //$NON-NLS-1$
> -        return sourceTypeMap;
> +    // Use for ComboBox Source Address Type. Preserve order (specially with localization)
> +    public static final ArrayList<String> getSourceTypeList() {
> +        ArrayList<String> sourceTypeList = new ArrayList<String>(4);
> +        sourceTypeList.add(JMeterUtils.getResString("web_testing_source_ip_hostname"));
//$NON-NLS-1$
> +        sourceTypeList.add(JMeterUtils.getResString("web_testing_source_ip_device"));
//$NON-NLS-1$
> +        sourceTypeList.add(JMeterUtils.getResString("web_testing_source_ip_device_ipv4"));
//$NON-NLS-1$
> +        sourceTypeList.add(JMeterUtils.getResString("web_testing_source_ip_device_ipv6"));
//$NON-NLS-1$
> +        return sourceTypeList;
>      }
>
>      public static final String DEFAULT_METHOD = HTTPConstants.GET; // $NON-NLS-1$
>
>

Mime
View raw message