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