On Wed, 8 May 2024 02:31:08 GMT, Alexander Matveev <almat...@openjdk.org> wrote:
>> modules/javafx.media/src/main/java/com/sun/media/jfxmedia/locator/HLSConnectionHolder.java
>> line 480:
>>
>>> 478: }
>>> 479:
>>> 480: class PlaylistLoader extends Thread {
>>
>> Having multiple top-level classes in the same source file is an
>> anti-pattern. Is there a good reason that these can't be nested static
>> classes? If not, then please make this change. You might be able to then
>> make the nested classes private, although there is no harm in leaving them
>> package scope.
>
> When these classes were nested classes I got issues with second instance of
> HLSConnectionHolder. If I remember correctly nested classes of second
> instance of HLSConnectionHolder were using fields of first
> HLSConnectionHolder instance. Maybe because I initiated second instance
> incorrectly. To avoid any such potential issues I decided to move away from
> nested classes. I would prefer to keep as is or better to move all nested
> classes under separate package (com.sun.media.jfxmedia.locator.hls).
Did you declare them as `static` (nested) classes? If not, then yes, they will
have the behavior you mentioned. A non-static "inner" class exists within an
instance of the outer class. A static "nested" class does not. Other than
scoping, a nested class behaves like a top-level class.
If you do want to keep them as separate top-level classes, then please move
each to its own file. I would not recommend creating a new package, though,
since that will involve more changes to make the classes and elements you need
public.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1435#discussion_r1593963759
