On Fri, 28 Oct 2022 17:57:30 GMT, Jim Laskey <jlas...@openjdk.org> wrote:
>> Enhance the Java programming language with string templates, which are >> similar to string literals but contain embedded expressions. A string >> template is interpreted at run time by replacing each expression with the >> result of evaluating that expression, possibly after further validation and >> transformation. This is a [preview language feature and >> API](http://openjdk.java.net/jeps/12). > > Jim Laskey has updated the pull request incrementally with one additional > commit since the last revision: > > Update TemplateRuntime::combine src/java.base/share/classes/java/lang/template/StringTemplate.java line 307: > 305: Objects.requireNonNull(fragment, "fragments elements must be > non-null"); > 306: } > 307: fragments = Collections.unmodifiableList(new > ArrayList<>(fragments)); I think using `List.copyOf()` is more efficient that `Collections.unmodifiableList(new ArrayList<>(...))` because there is no copy if the list is already created with List.of(). src/java.base/share/classes/java/lang/template/StringTemplate.java line 323: > 321: * @throws NullPointerException fragments or values is null or if > any of the fragments is null > 322: */ > 323: public static String interpolate(List<String> fragments, > List<Object> values) { This method also exists has a static method, having both is a bad idea because it makes StringTemplate::interpolate a compile error, the compiler has no way to know that it's the same implementation. src/java.base/share/classes/java/lang/template/StringTemplate.java line 354: > 352: * @implNote The result of interpolation is not interned. > 353: */ > 354: public static final StringProcessor STR = st -> st.interpolate(); Should be `StringTemplate::interpolate`. src/java.base/share/classes/java/lang/template/TemplateProcessor.java line 38: > 36: * that do not throw checked exceptions. For example: > 37: * {@snippet : > 38: * TemplateProcessor<String> processor = st -> { This is a good example of why having both way to describe a template processor of string, TemplateProcessor<String and `StringPorcessor` is a bad idea. src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 45: > 43: */ > 44: @PreviewFeature(feature=PreviewFeature.Feature.STRING_TEMPLATES) > 45: public final class TemplateRuntime { Why this class is public ? and it should be called `TemplateProcessors` linke all other classes in Java that store a bunch of static methods (Collections, Collectors, etc) src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 65: > 63: * @throws Throwable if linkage fails > 64: */ > 65: public static CallSite stringTemplateBSM( I wonder if this method should be moved to a class named `TemplateProcesorFactory` inside `java.lang.runtime`? Like the all the bootstrap methods recently added. src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 79: > 77: MethodType processorGetterType = > MethodType.methodType(ValidatingProcessor.class); > 78: ValidatingProcessor<?, ? extends Throwable> processor = > 79: (ValidatingProcessor<?, ? extends > Throwable>)processorGetter.asType(processorGetterType).invokeExact(); `ValidatingProcessor<?, ?>` should be enough ? No ? Not using a "? extends Throwable" here make the type unchecked. src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 88: > 86: * Manages the boostrapping of {@link ProcessorLinkage} callsites. > 87: */ > 88: private static class TemplateBootstrap { This class should be `final` src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 117: > 115: * Static final processor. > 116: */ > 117: private final ValidatingProcessor<?, ? extends Throwable> > processor; Use `ValidatingProcessor<?, ?>` here src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 145: > 143: private TemplateBootstrap(MethodHandles.Lookup lookup, String > name, MethodType type, > 144: List<String> fragments, > 145: ValidatingProcessor<?, ? extends > Throwable> processor) { Use ValidatingProcessor<?, ?> here src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 211: > 209: @SuppressWarnings("unchecked") > 210: public static <E> List<E> toList(E... elements) { > 211: return Collections.unmodifiableList(Arrays.asList(elements)); This is List.of(), please use List.of() instead ------------- PR: https://git.openjdk.org/jdk/pull/10889
