Enhance ItemList and make it usable for ComboBox/ChoiceBox

[manuel-mauky]

The package de.saxsys.mvvmfx.itemlist contains utils to bind a list of model elements to a ListView without leaking model details to the View.

However, in my opinion there are some problems at the moment:

• An observable list of Strings is used to represent the items in the View. This makes it hard to map back from a selected element to the actual model element. Example: Show a list of Persons with first and last name. To map from a Person instance to the label string is easy. However to map from the label String back to the actual Person instance can be quite hard. Maybe multiple persons have the same first and lastname? An alternative would be to use a unique identifier from the model elements as type of the list. If the Person entity has an id of type Long we would define a ListView<Long> in the UI. Of cause we don't want the ID to be visible in the ListView so we can define a CellFactory with a mapping from the ID to the Person to the Label value.
• SelectedStringList provides a ReadOnlyIntegerProperty for the selected index property. But in the ViewModel we most likely want to work with the currently selected model element. So we need to create a transformation logic for this by our self. One could even argument that in fact the information that there is something like a "index" is a view specific information that the ViewModel doesn't have to care about. What if the View changes the actual control to something that doesn't have an "index" but some other method of defining what is selected?
• the SelectedStringList provides imperative methods void select(int index) and void clearSelection() to manipulate the currently selected item. However this isn't convinient to use in the ViewModel. Instead we would like to have a bi-directional binding from the viewModel (ObjectProperty<Person> selectedPerson) to the ListView. As ListView only provides a read-only property for the selected value we can't easily create a bi-directional binding. Instead we would need to create listeners by hand.

I've worked on a new/improved prototype of utilities for this use case. This is what the user code could look like:

public class Person {
    @Id
    private Long id;
    private String firstname;
    private Strign lastname;
    // getter & setter
}

public class MyViewModel implements ViewModel {

    private ItemList<Person, Long> itemList = new ItemList<>(Person::getId);

    public MyViewModel() {
        // define how the item should be made visible in the ListView
        // by providing a Function from Modelelement to String
        itemList.setLabelFunction(person -> person.getFirstname() + " " + person.getLastname());

        List<Person> allPersons = ... // load from Backend

        itemList.getModelList().addAll(allPersons); // modelList works on actual model data

        ObjectProperty<Person> selected = itemList.selectedItemProperty();

        // we can set the selected person from code
        selected.setValue(...);
    }

    public ViewItemList<Long> itemList() {
        return itemList;
    }
}

public class MyView implements FxmlView<MyViewModel> {
    @FXML
    private ListView<Long> listView;
    @InjectViewModel
    private MyViewModel viewModel;

    public void initialize() {
        viewModel.itemList().connect(listView);
    }
}

The class ItemList is new. It has 2 generic type arguments: The type of the model and the type of the ID, in this case Long but it could be String too. The interface ViewItemList is implemented by ItemList. It provides the method connect(ListView<K>) where K is the generic type of the id.

In the View we only call the connect method and everything else is done by the framework. In the ViewModel we can work with a list of model data and get a property for the selected model instance. We don't need to take care for handling selection index.

What is cool about this approach is that we can provide additional connect methods for ComboBox, ChoiceBox and maybe even others. This could act as a general API for visualising "list data". Maybe #385 could be implemented this way too.

Open Points

• How does this relate to the utils in de.saxsys.mvvmfx.viewlist? This package contains helper to use mvvmFX Views as items in a ListView instead of only strings. Maybe this could be combined?
• While ComboBox and ChoiceBox only provide a SingleSelectionModel in the ListView multiple entries can be selected. At the moment the prototype only works with a single selected item. From my experience a single selection is by far the most common use case. For this reason it's ok to not have multi selection in the first place. However we should keep this in mind when defining the API
• For ComboBox and ChoiceBox there is sometimes the requirement for a "nothing selected" option. In my last project I've made the experience that this is not as easy to implement as it sounds. The util class should provide a clean way of implementing this.
• Is it problematic to use the ID of an entity in the View? Strictly speaking this would be a violation of the pattern because the ID belongs to the model. With the given utilities however the view doesn't use the actual ids. The only real information that is leaking to the View is the Type of the ID. If we later change the type of ID from Long to Integer we would need to update the Views. From my point of view this is acceptable. Another approach would be to use "Object" as ID type in the View. From the Views perspective the ID simply is "something that can be identified".

[sideisra]

:+1: Great idea. In my last projects we had a lot of discussions on how to implement this and about the separation of view and view model. Some colleages say that it is cleaner to have a complete separation which means to work only with strings in the view. In my opionion this does make this very hard and confusing. I prefer towork with model classes as generic types in the view (ListView) but i can understand the concerns. My be this id solution is a good balance.

You should also think about editable ComboBoxes. We often use an editable ComboBox enhanced with autocomplete functionality. Sometimes it is necessary to add new model objects to the modellist because the user entered a new element.

Sometimes we had trouble with changing the list while one element is selected. The SelectionModel of JavaFX is really problematic sometimes. A solution would be to clear the selection before any update and select the same element after the update (if its still there). I think such a behaviour would be achievable with this approach. Therefore ViewItemList could for example have a aroundCallback() method which takes a callback that work like a RequestFilter.

[manuel-mauky]

Hi Denny, thanks for the feedback. We have been using model classes as generic types in our last project too. But it feels wrong which is the reason I was working on this approach.

I will have a look at the editable ComboBox use case.

I don't fully understand your last use case. With the current prototype it's not possible to exchange the whole model list. Instead you can get the ObservableList with getModelList() and add or remove objects. I will try out what happens when the list is cleared and new Items are added. What do you mean with the callback? Can you post a (pseudo-)code example of what you think it should look like?

[sideisra]

Some implementations of the SelectionModel (e.g. SelectionModel of TreeTableView) contains Bugs so that exceptions are thrown when removing and adding elements from and into the list during one event. But there is also a scenario where such an aroundCallback could be useful. Given a scenario where a list of Person objects is shown in a TreeView. The TreeView shows the names only and they are orderd according to the name. When the user selects one person in the TreeView it is shown in a detail view. The user can change the name of the person and save it. Now the list in the TreeView has to be updated because the order is wrong now. When reordering the list, the edited person is removed from the list and added again at the correct position.

Problem: the SelectionModel selects the person right above the removed person but the desired bahaviour is to select the same person at the new position.

This scenario could be solved by an arroundCallback that works just like a ServletFilter (or JerseyFilter). example implementation:

class KeepSelectionCallback implements AroundCallback{
  public void doUpdate(UpdateChain chain){
    Long selectedElementId = treeView.getSelectionModel().getSelectedItem();
    //may be clear selection if necessary
    chain.doUpdate();
    treeView.getSelectionModel().select(selectedElementId);
  }
}

Another approach would be to offer two methods: addPreUpdateCallback and addPostUpdateCallback.

[manuel-mauky]

I've added a method replaceModelItems(Collection<T> items, boolean keepSelection).

It can be used to replace all model items. If the flag is true the old selected element (if any) will be cached. If the old element is present in the new item list it will be reselected afterwards. The implementation looks like this:

public void replaceModelItems(Collection<T> items, boolean keepSelection) {
    if(keepSelection) {
        T oldSelectedElement = selectedItem.get();

        getModelList().clear();
        getModelList().addAll(items);

        if(getModelList().contains(oldSelectedElement)) {
            selectedItem.setValue(oldSelectedElement);
        }
    } else {
        getModelList().clear();
        getModelList().addAll(items);
    }
}

Additionally a list changelistener makes sure that if the model list becomes empty the selected item is set to null.

I've extended the example so that the currently selected model item can be renamed by the user. After that all items are reloaded from the database. The item is still selected afterwards. I think this solves your use case doesn't it?

[sideisra]

Looks good.

When the element class implements equals and hashCode the check getModelList().contains(oldSelectedElement) may returns false because equals checks all data. It would be good when the caller could pass a Comparator to the method that is used to find the old selected element in the new list. This Comparator could for example be implemented to respect the id only.

[manuel-mauky]

Hi denny, instead of a comparator I would use a BiPredicate<T,T>, a function that returns a boolean for two given elements of T.

The code would look like this:

public void replaceModelItems(Collection<T> items, boolean keepSelection, BiPredicate<T,T> equaltyPredicate) {
    if(keepSelection) {
        T oldSelectedElement = selectedItem.get();

        getModelList().clear();
        getModelList().addAll(items);

        boolean contained = getModelList()
            .stream()
            .filter(element -> equaltyPredicate.test(element, oldSelectedElement))
            .anyMatch();
        if(contained) {
            selectedItem.setValue(oldSelectedElement);
        }
    } else {
        getModelList().clear();
        getModelList().addAll(items);
    }
}

[manuel-mauky]

I've started working on this issue again. Here are some updates:

• I've implemented the possibility to enable "no selection" mode. This way comboBox and choiceBox get an additional entry that represents "nothing is selected".
• I've removed the "keepSelection" flag in the replaceModelItems method because I think it should be the default behaviour anyway. If for some reason you like to remove the selection you can still unselect the currently selected item in your code.
• There is no BiPredicate argument for replaceModelItems at the moment. Instead only equals is used. I assume that the same is true for most of the collections implementations that are used internally by ListView, ComboBox and ObservableList etc. However, if there really is a use case for this, we could still provide such parameter in the future but until then we won't introduce this feature to keep the API small.

What still needs to be done:

• I've added unit tests for some of the implementation code. However, many parts of the code can't be easily tested with JUnit alone because of interaction with JavaFX controls like ListView, ComboBox and ChoiceBox. For this reason we should add TestFX test cases to check all features of the ItemList.
• Some parts of the code is already commented internally. However, JavaDoc is missing almost everywhere.
• Documentation for the feature needs to be added in the wiki
• At the moment the new ItemList is in the package newitemlist. This will most likely change in the future. However, at the moment I'm not sure how we will deprecate the old ItemList and introduce the new one.