On Sat, 8 Jan 2022 00:17:36 GMT, Marius Hanl <mh...@openjdk.org> 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

Reply via email to