On 17 February 2016 at 17:53, Graham Russell <[email protected]> wrote:
> Hi Sebb
>
> Thanks for having a look through the PR. I appreciate all the work you
> do with JMeter!
>
> I do not fully understand your argument. Do you have an example of
> issues to help me understand where explicit (un)boxing would help
> prevent a problem?
I already mentioned several reasons.
> My main reason for the PR was to reduce the amount of code to improve
> readability. In my opinion most explicit (un)boxing is just noise -
> happy to be persuaded otherwise.
>
> For example:
> What potential problems does the booleanValue() here avoid? I
> understand the need for the null check as you are right auto unboxing
> could hide a NPE.
>
> if (success != null) {
> if (success.booleanValue()) { ...
>
> vs.:
>
> if (success != null) {
> if (success) { ...
That looks very odd to me.
> If the reasoning was to make it even more obvious that it was an
> object and needed the null check, then, just as one example, this
> hasn't worked in the HtmlTemplateExporter class, the method
> EmptyGraphChecker.checkResult has
> supportsControllerDiscrimination.booleanValue() but no null check even
> though the method which created it could return null.
In which case the null check needs to be added.
> As another example, in a method which returns a long, I do not
> understand why the .longValue() is helpful, e.g.:
>
> public long getTimestamp() {
> return getData(long.class, CSVSaveService.TIME_STAMP).longValue();
> }
The point is that the boxing/unboxing warnings are useful for the
reasons I quoted originally.
If you suppress all the warnings, you don't get to find out about the problems.
Once you have decided that the correct datatype has been used, then
you can fix the warning by using explicit (un)boxing, thereby also
documenting that the use of the object is deliberate.
> Or why:
> Double.valueOf(((double) data.longValue() * 100 / errorCount))
>
> is preferred over:
>
> ((double) data * 100 / errorCount)
>
> I acknowledge you need to be mindful of NPE and auto (un)boxing in
> loops etc.
> (http://brian.pontarelli.com/2004/07/15/jdk-5-auto-boxing-best-practices/)
> but I currently do not see how being explicit about (ub)boxing helps?
Because the warnings help with exposing design issues and coding
errors, as per my original mail.
> I would very much appreciate any time you (or anyone else on the list)
> could spare to improve my understanding of this issue, via email or
> perhaps comment in github on specific changes and I would be happy to
> update the PR.
>
> Many thanks
>
> Graham
>
> On Mon, 15 Feb 2016 at 09:32 sebb <[email protected]> wrote:
>>
>> -1
>>
>> Auto boxing is generally a bad idea as it can hide poor design (wrong
>> choice of long/Long).
>>
>> It can also hide wrong choice of method (Long.parseLong/Long.decode)
>>
>> Implicit unboxing can hide potential NPE.
>>
>> About the only place where autoboxing is useful is in String.format,
>> but that should not be used as an excuse to allow it elsewhere.
>>
