On Sat, 16 Jan 2021 01:02:20 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> Some common logic is now extracted into methods. That's good. But much of >> the decoding logic still remains in String. I still think all of static >> methods can be moved to StringCoding directly as they are now while the >> private UTF-8 decoding constructor can be replaced with static method and >> moved to StringCoding. The only problem might be the public String >> constructor taking Charset parameter. But even here, passing a newly >> constructed mutable object to the method can be used to return multiple >> values from the method while relying on JIT to eliminate object allocation. >> Do you think this code belongs more to String than to StringCoding? > >> Do you think this code belongs more to String than to StringCoding? > > I consider StringCoding an implementation detail of String, so I'm not sure > there's (much) value in maintaining the separation of concern if it is cause > for a performance loss. While encapsulating and separating concerns is a fine > purpose I think the main purpose served by StringCoding is to resolve some > bootstrap issues: String is one of the first classes to be loaded and eagerly > pulling in dependencies like ThreadLocal and Charsets before bootstrapping > leads to all manner of hard to decipher issues (yes - I've tried :-)). This looks much better now. Maybe just one small suggestion: `java.lang.StringCoding#lookupCharset` is used in two places and in both places the same exception handling/rethrowing logic is wrapped around the invocation. So you could move that logic into the lookupCharset method which would simplify call sites. You could even get rid of String.lookupCharset method that way. ------------- PR: https://git.openjdk.java.net/jdk/pull/2102
