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,  <[email protected]> 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$
>
>

Reply via email to