On Mon, 3 Nov 2025 13:23:46 GMT, Mikhail Yankelevich <[email protected]> wrote:
>> 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. The `implInit()` method called from chooseProvider() is initializing the chosen provider/implementation. It's called twice in chooseProvider and can't be called from the public `init()` methods because the `spi` object may not have been found yet. My change can't be added to `implInit()` because the serviceIterator needs to be valid (i.e., hasNext() == true) before the `spi` object is found and implInit only operates on a valid spi object. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28004#discussion_r2557535392
