On 29/07/2013 22:56, Jeremy Boynes wrote:
> On Mon, Jul 29, 2013 at 9:51 AM, Mark Thomas <[email protected]> wrote:
>
>> On 29/07/2013 01:01, [email protected] 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: [email protected]
For additional commands, e-mail: [email protected]