On Tue, 8 Jul 2025 21:33:00 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Alexander Zuev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove explicit jresult declaration
>
> modules/javafx.graphics/src/main/native-glass/mac/a11y/AccessibleBase.m line
> 113:
>
>> 111: GET_MAIN_JENV;
>> 112: if (env == NULL) return NULL;
>> 113: jresult = (jobject)(*env)->CallLongMethod(env, [self
>> getJAccessible],
>
> not an expert, but do we really need to initialize `jresult` to NULL in line
> 110?
> can the variable be declared in line 113 ?
Well this pattern was used to initialize the result value to the default return
value so if assignment does not happen due to the Java exception we have
something meaningful to return but in case of NULL it is not really required
because it will produce the same result anyways.
> modules/javafx.graphics/src/main/native-glass/mac/a11y/AccessibleBase.m line
> 191:
>
>> 189: id p = [self requestNodeAttribute:@"AXPosition"];
>> 190: id s = [self requestNodeAttribute:@"AXSize"];
>> 191: if (p == NULL || s == NULL) {
>
> not a big issue, but would it be (marginally) better to bail out early if p
> == NULL ?
But that would require two conditions - one for position and one for size -
with the same logic which would look not that compact and pretty.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1840#discussion_r2194014740
PR Review Comment: https://git.openjdk.org/jfx/pull/1840#discussion_r2194018393
