You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by "FSchumacher (via GitHub)" <gi...@apache.org> on 2023/05/03 19:23:32 UTC

[GitHub] [jmeter] FSchumacher opened a new pull request, #5876: Use JMeterProperty#intValue for loop count directly

FSchumacher opened a new pull request, #5876:
URL: https://github.com/apache/jmeter/pull/5876

   ## Description
   
   This will get rid of the roundtrip over a String in case, the prop is already an IntegerProperty.
   
   ## Motivation and Context
   
   Came up in the discussion about #5875 and looks like an easy first fix.
    
   ## How Has This Been Tested?
   
   Should be covered by the unit tests already
   
   
   ## Types of changes
   - Bug fix (non-breaking change which fixes an issue)
   
   ## Checklist:
   <!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
   <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
   - [x] My code follows the [code style][style-guide] of this project.
   
   [style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [jmeter] vlsi merged pull request #5876: Use JMeterProperty#intValue for loop count directly

Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi merged PR #5876:
URL: https://github.com/apache/jmeter/pull/5876


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [jmeter] FSchumacher commented on pull request #5876: Use JMeterProperty#intValue for loop count directly

Posted by "FSchumacher (via GitHub)" <gi...@apache.org>.
FSchumacher commented on PR #5876:
URL: https://github.com/apache/jmeter/pull/5876#issuecomment-1536374965

   I looked at `ThroughputController` and it is not easily fixed by using this approach, as it relies on a different default value.
   The `AbstractProperty#getFloatValue` catches `NumberFormatException`s and returns `0`. So, using it directly would lead to a different default value in case of a broken value.
   How did you search for those occurrences? I had to look a bit longer to find line 600 in JMSSampler, but there are surely more? Line 600 is a good candidate to replace, so I will extend this PR with it. Feel free, to push any changes, too.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [jmeter] vlsi commented on pull request #5876: Use JMeterProperty#intValue for loop count directly

Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on PR #5876:
URL: https://github.com/apache/jmeter/pull/5876#issuecomment-1534383349

   Could you please fix the other getStringValue->parseInt cases?
   For instance: `ThroughputController`, `JMSSampler`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [jmeter] FSchumacher commented on pull request #5876: Use JMeterProperty#intValue for loop count directly

Posted by "FSchumacher (via GitHub)" <gi...@apache.org>.
FSchumacher commented on PR #5876:
URL: https://github.com/apache/jmeter/pull/5876#issuecomment-1536479429

   If you look at that method, you will see, that it assumes a default value of `1` and uses that in case of a `NumberFormatException`. If we use `JMeterProperty#getIntValue` we would never get that exception, but a default value of `0`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [jmeter] vlsi commented on pull request #5876: Use JMeterProperty#intValue for loop count directly

Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on PR #5876:
URL: https://github.com/apache/jmeter/pull/5876#issuecomment-1536383032

   I used "find usages" for `getStringValue`, and then I filtered the entries that parse the result as integer.
   
   I think this looks like https://github.com/apache/jmeter/blob/b6d11d79d905d0c099732bb928d2372fd1388981/src/components/src/main/java/org/apache/jmeter/control/ThroughputController.java#L129-L131 is pretty much like "get property as int" to me.
   
   I'm not sure what to do with `getFloatValue`, however, it looks like `getStringValue` + `parseFloat` should also be replaced with `getFloatValue`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org