-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Mark,
On 5/8/19 12:20, Mark Thomas wrote: > On 08/05/2019 17:16, Christopher Schultz wrote: >> All, >> >> While working on the sort-file-listing enhancement over the >> weekend, I looked in the Tomcat code to see if we already had a >> method to split a string by a single character. The closest >> method I found was org.pache.jasper.compiler.JspUtil which splits >> on a string, so I rejected it as being a bit too general-purpose >> for my needs. >> >> But it got me looking into the implementation of the method, and >> it looks a little sketchy. >> >> First, it uses a java.util.Vector. Yep, a Vector. Remember >> those? Synchronized everything? The Vector is not used outside of >> the method at all -- it's just local and temporary. So, the >> obvious conclusion is that it should be changed to ArrayList, >> right? >> >> But, modern JVMs are fairly good at (a) processing >> contention-less locks more quickly and (b) identifying objects >> which cannot possibly have lock-contention and simply skipping >> the locking. >> >> What's the right move in these cases? Should we change Vector -> >> ArrayList and "help-out" the compiler and runtime, or should we >> leave the code alone because "it's fine"? >> >> Similar arguments can be made for using StringBuilder over >> StringBuffer, etc. >> >> WDYT? > > I think the code is cleaner and easier to understand when the code > matches the intention rather than when the code plus some expected > compiler optimisation match the intention. So I'd say +1 to making > these sorts of changes. This is kind of my thought as well. I also feel like code coming from Apache ought to be an example of the "right way of doing things" wherever possible. Obviously, that's not a goal of many projects both within and outside the ASF, but I kind of feel like we should be able to point to Tomcat code and say "yes, this is how it should be done". If it doesn't look nice, then it should be changed. I'm guessing that's one of the reasons you went through all the code and changed try/finally to try-with-resources and Foo<Bar> baz = new Foo<Bar> to Foo<Bar> baz = new Foo<>() everywhere. I think this is kind of the same. Back to JspUtil.split. It's private and used exactly one place. AND it's only ever called with a single-character String value. So actually the use-case DID meet my needs for string-splitting for the sorting. Perhaps I'll re-factor that method a little and then use it across these two use-cases. - -chris -----BEGIN PGP SIGNATURE----- Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/ iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAlzTBCsACgkQHPApP6U8 pFjXaxAAmStuVV4R5xLXOuI7+svzqZ9qPBiDMa/KjBk2lvFBoZ7H1pg8OZcLH2qO z+iGmf7IvdJ13SBwQfaGGZ556iJGjFkx3IX6+sC8jeBJt0se2BKU8UrsKlesYnnw ZWaz9+gfg0Ab/N0l7Oh+BtHAdk60l1Lhg+3Pb4owHkJZj40VeIe59c7iF2OshBh9 PZOtek1TJkE1Mi2XmQKAqA6MJ8LDdUIIuFj+p4pWy+OOOzzywTvYNSlPe8Ozo8u9 9EpXis8wi66DMGMQXLtCbC9TW1z2DAV0r5lHfCuX1tdhvyDb9PvBKnL9SdIEB1Tf 4HUjnQ+XCUlXpsP1GjKD1W9v3z+dxuWbFoA/tZgtagc3+nsizkEwX7EFMAa9nfZq 7dYwbMIi6ypZZbDfzYTw3VD73KUZa8IRQd6zQBU8jYp/gT1sb4uSLzPhV73IyHDQ tzdjtrwUzk0TPFJR7LgaOJBuGfIwh3gN19c+mNOgft6l7OBsu8rxXH+B7haoPkVD u3KBWpvXhKTY6QWoUPQt3wgYysWUGjpigPJyFx5ndk5zVl5MSB0AXSZBEkBRuAG+ yEfumxHKtVy2OFLFgw65Wc2P6CZcYLIdGu5qccKFiyD/f8zh4vdeWcfWtsG8Siye sCXJRBg4CwysWzVusTTus6ajb/MkMtOsj7NkU7f24o3KQ+EyIAY= =/PKS -----END PGP SIGNATURE----- --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
