On Sat, 8 Jan 2022 00:17:36 GMT, Marius Hanl <[email protected]> wrote:
> This PR fixes a bunch of NPEs when a null `SelectionModel` or `FocusModel` is
> set on a `ListView`.
>
> The following NPEs are fixed (all are also covered by exactly one test case):
> NPEs with null selection model:
> - Mouse click on a `ListCell`
> - SPACE key press
> - KP_UP (arrow up) key press
> - HOME key press
> - END key press
> - BACK_SLASH + CTRL key press
>
> NPEs with null focus model:
> - SPACE key press
> - Select an items: getSelectionModel().select(1)
> - Clear-Select an item and add one after:
> listView.getSelectionModel().clearAndSelect(1); listView.getItems().add("3");
Was it considered to not allow these models to be set to `null` instead? Or to
use a Null Object? I'd much prefer this instead of polluting the code with
`null` checks on every access of these models.
Furthermore, the API makes no mention what is supposed to happen when these
models are set to `null`, so this seems undefined at this time. If `null` is
supposed to be valid then I think the documentation should clearly state what
this means. Having no focus model for example should probably mean that
nothing can ever get focus and that the focused item is always null and index
is always -1. Instead of adding null checks to achieve this however I'd use a
`NullFocusModel` instance as it is easy to forget a `null` check (also when
making future changes).
Same goes for the selection model; if set to `null` it should not allow any
kind of selection, which again seems best achieved by installing a model that
ignores all such actions and always returns empty values.
-------------
PR: https://git.openjdk.java.net/jfx/pull/711
