On Tue, 12 May 2020 17:58:09 GMT, John Hendrikx <[email protected]> wrote:
> I'm fine with doing a fix, but I need to know which one. Avoiding NPE's and > silently doing nothing is IMHO not very > desirable as this will give the user of the API no feedback that something > went wrong. > So I would prefer to fix this by documenting that these cases will result in > a NPE. > > The `bind` method has a similar issue -- it doesn't check its array elements > for `null`, and will throw a NPE when > attempting to add a listener to `null`. Again, I would just document the NPE > so what is clearly a mistake doesn't go > unnoticed. I think that checking the array elements doesn't help much because by the time you can check them they will already be used, and that will throw an NPE implicitly. `bind` is no-op for `null` or 0-length arrays, but should have probably throw an NPE on the `null` case. The 0-length check saves creating the `observer`, so I think it makes sense. `unbind` should probably only throw an NPE on `null` arrays, but that happens implicitly anyway, so maybe there is no change needed unless it's for clarity because we can add a custom error message. ------------- PR: https://git.openjdk.java.net/jfx/pull/198
