So Sonar (Java code style checker) is telling me to return immediately instead of first assigning the results to a variable:
ArrayList<string> strings = ...

{Some long running logic that populates the list}

String x = String.join(strings);
return x;

Declaring x is bad apparently... but I disagree...

Am I not understanding something here?

The upside out this is you can breakpoint it and well you meet want to add additional logic later while you find a bug while debugging...

I guess it would be noticeably slower but a few seconds... If I were to call it 1 billion times?

@runfrodorun wait but x is just a reference, a name. It's not the actually data itself.

At most it costs 8 more bytes in memory and a few ms and then when it exits those 8 bytes are marked as cleanup.

Performance impact... negligible.
Time saved when debugging...a lot.

@runfrodorun strings are immutable.

All Java primitives are passed by value. IF want to pass by reference you use AtomicReference<>

@runfrodorun forgot your passing case.... I changed a to a class static string

@runfrodorun and this is pass by reference.

Actually, primitives i think are the same in most languages. They pass by value but different languages use different keywords to pass by reference.

Now Objects are always passed by reference as you can see, AtomicReference is an object that holds a primitive.

Keep in mind that Sonarqube is a tool to help you. It should be no problem to change this rule, if you have a good reason for it.

But besides that, you can also keep your code style and still satisfy Sonarqube.

final String x = String.join(strings);
LOG.debug("joined strings: {}");
return x;

Now you can gather information by adjusting the log level to debug your class and the variable is used before being returned.

Through the replacement with debug you are also not wasting performance to create any unnecessary debug strings that you might not use.

@Makenshi but why make the rule at all? How does it fit in as a "best practice"?

The only case this rule makes any sense is if you call the function so many times the creation of the pointer becomes noticable.

The other real case is in a recursive function... And well unless you're using Scala, that'll cause a StackOverflowException first...

This has nothing to do with optimization, and everything to do with code style. The generated code should be exactly the same.

The question is why you'd rather define a variable when all you're going to do with it is return it immediately? The code is (slightly) less readable than just returning the result.

@configurator say your debugging/testing so breakpoint on the return line, how do you inspect what's being returned?

if the return executes a function now you gotta run it again after **assigning it to a variable and changing the return to return the variable**

And then for code style sake you're going to remove it and return directly again... because you're sure now you got it perfect... until it breaks again and now you repeat... All because you shouldn't have a temp variable?

Which btw pretty much costs nothing to create.

so actually this rule seems to be an example of "pre-optimization is the root of all evil"

@billgates Not really. You assume that the code would break in the first place. But it has not yet. In fact leaving the reference for future debugging is premature optimization.

Normally you would write an unit test to cover this method. You would rarly need to debug it by hand.

And even if you do, every major Java IDE has the ability to execute live code while connected with a debugger. IntelliJ Idea has full blown support for it, while NetBeans and Eclipse allowed for such executions through watch expressions.

@Makenshi yes but if you just return, you break point on the outside functiom instead. You lose the call stack and then to go back you have to move the debug pointer and call the whole function again.

Unit tests yes in theory... In practice maybe but closer to no.

Say your calling a DAO find()

var results = longDbQuery()
//Some other stuff done on results
return transform(results)

From the caller, I can't see the result of transform until it exits... but it there is an issue, I gotta run the whole doubting again.

@billgates I find that to be a silly reason to leave the code in the less readable state, but if it works for you you should disable the rule in your validator.

@billgates If you call a function last you can simply step into that function and look at the last return statement.

As described in the last paragraph of my previous post, you can also straight execute the last line of code live while debugging. You would not lose any debug information while doing so.

Also why would this not work in a Unit- or Integration test? I'm have worked and I'm working at the moment with several projects that contain tests to maintain a certain quality of code. But I digress.

You asked why Sonarqube hints at this being bad practise. The explanation is:
* It produces one unnecessary reference and line of code
* It's no real performance issue but merely to keep the code a little bit more tidy
* It does not hinder debugging, as you can:
** Step into the last called function
** Execute the last line via "live coding" through the JVM debugger