On Mon, 3 Nov 2025 13:13:34 GMT, Matthew Donovan <[email protected]> wrote:
>> src/java.base/share/classes/javax/crypto/KeyAgreement.java line 377:
>>
>>> 375:
>>> 376: if (!serviceIterator.hasNext()) {
>>> 377: serviceIterator =
>>> GetInstance.getServices("KeyAgreement", algorithm);
>>
>> Wouldn't it cause a change in behaviour of this method unnecessarily? It
>> seems to me it might be a bit better to do this in the `init` methods
>> calling `chooseProvider`. What do you think?
>
> I don't understand what you mean about changing the behavior of the method.
> The behavior has to change somewhere, doesn't it?
>
> I don't like the idea of moving this to the init() method because the code
> would have to be duplicated in two places and the init methods don't
> currently have any knowledge of how providers are chosen. it's unlikely but
> if the chooseProvider() implementation is changed that could force an
> unnecessary change to the init() methods.
I was thinking more about why is it done in `chooseProvider`, this makes the
method name inaccurate, as now you're initialising in the method itself.
Actually, after looking at it the second time, I can see that the method
already calls another `init` method and this is a bit confusing to me. It seems
to me that it would be more appropriate to move both initialisations to a
private helper method, call them from both `init`s and have `chooseProvider`
only to choose provider.
I'm fine if you prefer to keep it as it is to minimise changes, but I still
think it would be beneficial in the end.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28004#discussion_r2486479928
