On Fri, 16 Oct 2020 14:00:38 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> Maybe I'm being too pedantic, but is the use of the term _copy_ for the
>> withers overly prescriptive, and possibly
>> limiting an implementation? For example, for `withDelimiter` does _copy_
>> preclude such an implementation:
>> /**
>> * Returns a copy of this {@code HexFormat} with the delimiter.
>> * @param delimiter the delimiter, non-null, may be empty
>> * @return a copy of this {@code HexFormat} with the delimiter
>> */
>> public HexFormat withDelimiter(String delimiter) {
>> + if (delimiter.equals(this.delimiter))
>> + return this;
>> return new HexFormat(delimiter, this.prefix, this.suffix,
>> this.digits);
>> }
>
>> Maybe I'm being too pedantic, but is the use of the term _copy_ for the
>> withers overly prescriptive, and possibly
>> limiting an implementation? For example, for `withDelimiter` does _copy_
>> preclude such an implementation:
>> ```
>> /**
>> * Returns a copy of this {@code HexFormat} with the delimiter.
>> * @param delimiter the delimiter, non-null, may be empty
>> * @return a copy of this {@code HexFormat} with the delimiter
>> */
>> public HexFormat withDelimiter(String delimiter) {
>> + if (delimiter.equals(this.delimiter))
>> + return this;
>> return new HexFormat(delimiter, this.prefix, this.suffix,
>> this.digits);
>> }
>> ```
>
> Since the instances are immutable, it seemed useful to emphasize that only
> the instance returned has the requested
> change. Discarding the return value was incorrect programming. The original
> instance is not modified. The "copy"
> phrasing was used throughout java.time. Since reference equality is
> specifically dis-allowed for value-based objects,
> there is no way to discover a difference between a copy and the original with
> the same parameters.
Thanks for the PR.
For TestNG asserts as a point of personal style, I eschew the static imports
for asserts.
The correction for `<A extends Appendable>` is already in the works.
-------------
PR: https://git.openjdk.java.net/jdk/pull/482
