On Tue, 25 Mar 2025 14:20:24 GMT, Luca Kellermann <d...@openjdk.org> wrote:
>>> Comments on visual noise and side effects in `toString`. >>> >>> `renderWrapped` is clever for a single stable value, but it makes for a >>> very noisy display string, with confusing doubly-nested `[]`, for composite >>> stable values. I'm talking about `StableFunction` mainly, I guess. >>> >>> I suggest omitting the inner `[]` for such composites. A simple boolean on >>> `renderWrapped` will do that trick. In that case, `renderWrapped` has the >>> job of either presenting a fixed (recognizable) sentinel string, or else >>> forwards, without further editorial comment, to the `toString` of the >>> contained value. >> >> The `toString()` for `StableValue` is inspired by `Optional` which works in >> the same way by adding `[ ]` around the contents. Any more thought in the >> reviewer community on how we should handle this? > >> > Comments on visual noise and side effects in `toString`. >> > `renderWrapped` is clever for a single stable value, but it makes for a >> > very noisy display string, with confusing doubly-nested `[]`, for >> > composite stable values. I'm talking about `StableFunction` mainly, I >> > guess. >> > I suggest omitting the inner `[]` for such composites. A simple boolean on >> > `renderWrapped` will do that trick. In that case, `renderWrapped` has the >> > job of either presenting a fixed (recognizable) sentinel string, or else >> > forwards, without further editorial comment, to the `toString` of the >> > contained value. >> >> The `toString()` for `StableValue` is inspired by `Optional` which works in >> the same way by adding `[ ]` around the contents. Any more thought in the >> reviewer community on how we should handle this? > > I think the comment just proposes that this code > > var f = StableValue.intFunction(3, i -> i); > f.apply(1); > System.out.println(f); > > should print > > StableIntFunction[values=[.unset, 1, .unset], original=...] > > Right now it prints this: > > StableIntFunction[values=[.unset, [1], .unset], original=...] > > The `toString` implementation for `StableValueImpl` itself seems fine. Thank you, @lukellmann, for your comments. Some of them were very useful! I will address them _after_ integrating the PR. ------------- PR Comment: https://git.openjdk.org/jdk/pull/23972#issuecomment-2830400309
