ChristopherSchultz commented on pull request #321:
URL: https://github.com/apache/tomcat/pull/321#issuecomment-656880783
> Bad idea on adding concatenation (rest is good).
I don't see any "added" concatenation. Some uses of `StringBuilder` have
been replaced with string concatenation which is a win for several reasons:
1. It's much easier to read + debug
2. The compiler can convert to using StringBuffer (old) StringBuilder
(newer) or StringConcatFactory (newest) instead of using the latest technique
some programmer most-recently read about
> That causes lots of extra object creation needlessly.
Citation needed. The compiler converts this:
return "foo" + 25 + "bar";
Into roughly this:
StringBuilder sb = new StringBuilder("foo");
sb.append(23);
sb.append("bar");
return sb.toString();
So going backward doesn't add anything.
> Concatenation is long known to be a problem.
A problem which the compiler is perfectly capable of handling. This part is
about readability and maintainability, which are being improved by the patch
IMO.
> This PR is doing 2 things, fixing real issues and then adding unnecessary
concatenation in others.
Please be specific: point to a single place where concatenation is being
added where it did not previously exist.
> There are better ways to do that without string builder
Such as?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
