RFR: 8325851: Hide PassFailJFrame.Builder constructor
Alexey Ivanov
aivanov at openjdk.org
Mon Mar 11 22:29:12 UTC 2024
On Mon, 11 Mar 2024 22:09:09 GMT, Phil Race <prr at openjdk.org> wrote:
>> The `Builder` class in `PassFailJFrame` is public and has a public constructor. At the same time, a better design would be to hide all the Builder constructors and rely on the `builder()` method which returns an instance of the `Builder` class.
>>
>> This PR makes the constructor of the `Builder` class private so that it's not accessible directly.
>>
>> Use `PassFailJFrame.builder()` to get an instance of the builder and configure parameters of `PassFailJFrame`.
>>
>> I updated all the tests which used the constructor directly by calling `new PassFailJFrame.Builder()`.
>>
>> I converted a few tests to use the `testUI`, which greatly simplifies the test code. This change also removes flickering of the test UI.
>
> test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 1070:
>
>> 1068: }
>> 1069:
>> 1070: public Builder title(String title) {
>
> So what about this one ? Are too many tests using it ?
What about them? All the methods in `Builder` return `Builder`, that's perfectly fine.
The goal of this PR is to make impossible to write
new PassFailJFrame.Builder();
which is replaced with
PassFailJFrame.builder();
In the former case, the test explicitly depends on the fact that the builder pattern implementation is in the `PassFailJFrame.Builder` class.
In the latter case, I can change the `builder()` method to return another class. As long as that other class has the same set of methods, like `instructions` and title`, which return the same object, nothing breaks. Encapsulation.
With this change, all the existing tests continue to run. Those tests which failed to compile because of a breaking API change are updated in this PR.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18206#discussion_r1520497593
More information about the client-libs-dev
mailing list