On Mon, 2 Mar 2026 22:20:41 GMT, Phil Race <[email protected]> wrote:

> java.beans.beancontext.BeanContextSupport has add() and remove() methods 
> which synchronize on their arguments.
> 
> 1) This won't work with value types such as Integer, Byte, Short, etc ..
> 2) It seems unnecessary. The sychronization is already wrapped in       
>  synchronized(BeanContext.globalHierarchyLock) {
> ..
> }
> 
> The diff looks big but that's because it has to ex-dent a bunch of lines. It 
> is really just a 4 line change.

Looks good to me, except for three consecutive blank lines in `add`.

src/java.desktop/share/classes/java/beans/beancontext/BeanContextSupport.java 
line 425:

> 423:             BeanContextChild  bccp = null;
> 424: 
> 425: 

Are *two* blank lines necessary here? One should work just fine.

src/java.desktop/share/classes/java/beans/beancontext/BeanContextSupport.java 
line 488:

> 486: 
> 487: 
> 488: 

This leaves *three* consecutive blank lines. Two could be reasonable, provided 
that you left two blank lines at the start of the block, but three are too much.

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

Marked as reviewed by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/30008#pullrequestreview-3883928712
PR Review Comment: https://git.openjdk.org/jdk/pull/30008#discussion_r2879467509
PR Review Comment: https://git.openjdk.org/jdk/pull/30008#discussion_r2879477027

Reply via email to