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

Reply via email to