refactored by concatenating String to be appended to StringBuilder

[Shivangip285]

Refactor

[karussell]

Thanks for your contribution. Unfortunately I do not understand the intend of the refactoring. What do you think has improved with it? Why e.g. is append replaced with String concatenation?

PS: I fixed the build in the master branch

[Shivangip285]

Sure, actually I made 3 commits with following reasons. 1. refactored by concatenating String to be appended to StringBuilder <https://github.com/graphhopper/geocoder-converter/commit/ac9141fef41c43b59218f7b4388c94e5d45454a6>- I had preferred concatenation of static String with dynamic String followed by append, over multiple append for code optimization purpose, if you observe those multiple appends were appending static or constant String value like ", "using append . 2. Refactored by removing redundant variable rsp <https://github.com/graphhopper/geocoder-converter/commit/5fe5127076f03162cf94a5f707e514cd42d82c67>-here variable was defined and assigned and then returned in next line by function, to optimize this I had combined returned the value directly instead of assigning to a redundant variable 3. Refactor by replacing if block with optimized Math.min() <https://github.com/graphhopper/geocoder-converter/commit/76e9f6758e89d03c488f2821b155751cc0e7fc22>- this is optimized replacement of 3 line of if block with Math.min(). I had started with small refactoring for code optimization and clean coding, these individual changes might not have a very high effect on performance but, collectively they will help improving performance and keeping the code clean.

…

On Thu, Nov 3, 2022 at 10:32 PM Peter ***@***.***> wrote: Thanks for your contribution. Unfortunately I do not understand the intend of the refactoring. What do you think has improved with it? Why e.g. is append replaced with String concatenation? PS: I fixed the build in the master branch — Reply to this email directly, view it on GitHub <#68 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AZPVVCINMJKSGUUR4XOOBLDWGPVZPANCNFSM6AAAAAARR6NHIA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[karussell]

I had preferred concatenation of static String with dynamic String followed by append,

In non-performance critical code this is indeed better readable, but I would keep it as it is here. Also you shouldn't mix append and +.

collectively they will help improving performance and keeping the code clean

improving performance is only possible if you measure your results. You shouldn't change code only because you think it is faster (e.g. use JMH or apache bench). Also from my experience it does not matter if you use append or + although append was slightly faster in previous JVM versions, but I'm unsure why + should be faster. Also a variable like rsp that is immediately returned is not a performance problem.

this is optimized replacement of 3 line of if block with Math.min()

Ok