On 29/07/2013 22:56, Jeremy Boynes wrote: > On Mon, Jul 29, 2013 at 9:51 AM, Mark Thomas <ma...@apache.org> wrote: > >> On 29/07/2013 01:01, jboy...@apache.org wrote: >>> Author: jboynes >>> Date: Sun Jul 28 23:01:23 2013 >>> New Revision: 1507870 >>> >>> URL: http://svn.apache.org/r1507870 >> >> Generally, line lengths should be limited to 80 characters. I know not >> all of the Tomcat code does that but new code should. >> > > I'll set up style rules and fix that.
Note the "Generally". It isn't a strict rule. More that we aim for 80 but use judgement as to when to bend that. >>> +public class WebappServiceLoader<T> { >>> + private static final String LIB = "/WEB-INF/lib/"; >>> + private static final String SERVICES = "META-INF/services/"; >>> + private static final Charset UTF8 = Charset.forName("UTF-8"); >> >> Should use B2CConverter.UTF_8 > > I try to avoid dependencies and this would be the same instance from > Charset's cache so I don't see any memory benefit. Am I missing something > here? Not a big deal in this case but generally I try to use constants where we have them. The dependency in this case isn't a concern as multiple dependencies exist between the relevant JARs and I don't ever see is trying to remove those dependencies. The bigger issue to watch our for is this one: https://issues.apache.org/bugzilla/show_bug.cgi?id=51400 It was work with that performance issue that lead to creation of the constants in B2CConverter. I thought Java 7 had defined some standard Charsets but couldn't find them when I wrote my previous e-mail. I've since found them: java.nio.charset.StandardCharsets We should be using these in Tomcat 8. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org