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