On Fri, 25 Apr 2025 07:06:05 GMT, Alan Bateman <[email protected]> wrote:
>>> What should we replace it with? Do you have any suggestions?
>>
>> The wrapper classes were needed when there were was a mix of synchronized
>> and j.u.concurrent locks in use. With JEP 491 integrated it meant that the
>> java.io classes could be changed back to use synchronized. Yes, we could do
>> some cleanup in Throwable too. Changing PrintStreamOrWriter to be an
>> interface should be fine but the rest of these changes in this PR doesn't
>> seem necessary. As others have already asked, I think it would be useful to
>> understand what issue you are running into and why you want to change this
>> code.
>
>> @AlanBateman I have modified it to use interface + record. Is this what you
>> want?
>
> I don't object to changing it to interface + record but it feels more like
> needless code churn. I really disliked the next version that used
> printStackTrace0(Object printer) as it immediately invites another re-write.
The record has fewer source lines but doesn't the generated record class have a
getter, toString(), hashCode() and equals(Object) which are not needed here?
What about using a lock parameter and a method handle to pass in the required
features of PrintStream and PrintWriter:
public void printStackTrace(PrintWriter pw) {
printStackTrace(pw, pw::println);
}
public void printStackTrace(PrintStream ps) {
printStackTrace(ps, ps::println);
}
private void printStackTrace(Object lock, Consumer<Object> println) {
synchronized(lock) {
...
println.accept(this);
...
}
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24795#discussion_r2059745913
