RFR: 8360037: Refactor ImageReader in preparation for Valhalla support [v12]

David Beaumont duke at openjdk.org
Fri Aug 1 13:45:02 UTC 2025


On Tue, 22 Jul 2025 15:26:22 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> True, though would you prefer to change the comment ("by default this method throws... and is overridden by directory subclasses...") or the implementation of things like `getChildNames()` so they call `isDirectory()` ?
>> 
>> Personally I dislike this "test and call" approach and would rather have restructured the API to be more object-oriented, and have callers use a more structured dispatch mechanism (but this would incur cost of lambdas etc.), but that's a really disruptive change.
>> 
>> Alternatively a type token/enum of some sort could be used to define node type. This is all internal/trusted API though, so I'm happy with trusting that things "do what they say" (it's going to be really obvious if something claims to be a directory and then throws when asks for its child names, and that's almost exclusively why anyone calls isDirectory() to start with).
>> 
>> So in summary, apart from maybe tweaking the comment slightly, I think this is fine as is.
>
> I would change the comment.

Comment changed, though note that I can't @link to Directory because it's a non-visible type.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26054#discussion_r2248026262


More information about the core-libs-dev mailing list