On 08/23/2017 01:52 PM, Claes Redestad wrote: > What I wasn't sure about is *when* the String.valueOf should happen, but as > makeConcatWithConstants specify "If necessary, the factory would call > toString to > perform a one-time String conversion" then I think we could (should?) do this > at > our earliest convenience, which might even be in the RecipeElement > construction:
Yes, I don't see why not. > http://cr.openjdk.java.net/~redestad/8186500/jdk.02/ Hm! *) Seems that due to cnst being an (autoboxed) reference, isPrimitive is always false on this path? 332 Object value = Objects.requireNonNull(cnst); 333 if (!value.getClass().isPrimitive()) { 334 this.value = String.valueOf(cnst); 335 } else { 336 this.value = value; 337 } ...so that value is always String after this? ...and that avoids String conversion on RecipeElement.getValue() paths completely? *) If not, these are replaceable with String.valueOf: 937 String s = (cnst != null) ? cnst.toString() : "null"; ... 998 String s = (cnst != null) ? cnst.toString() : "null"; > Reworked the test to be easier to add constant types. Null constants are > specified > to throw NPE, so the test now checks that (this might be a needlessly strict > thing > to specify, but changing the method specification is out of scope here, I > think). Ah, good! I was not completely reckless with "null" checking after all! Thanks, -Aleksey
