diff --git a/mvvmfx/src/main/java/de/saxsys/mvvmfx/utils/mapping/ModelWrapper.java b/mvvmfx/src/main/java/de/saxsys/mvvmfx/utils/mapping/ModelWrapper.java index 13561f253..80107fbd9 100644 --- a/mvvmfx/src/main/java/de/saxsys/mvvmfx/utils/mapping/ModelWrapper.java +++ b/mvvmfx/src/main/java/de/saxsys/mvvmfx/utils/mapping/ModelWrapper.java @@ -204,9 +204,6 @@ * * * - * - * - * * @param * the type of the model class. */ @@ -220,8 +217,9 @@ public class ModelWrapper { /** * This interface defines the operations that are possible for each field of a wrapped class. * - * @param - * @param + * @param target type. The base type of the returned property, f.e. {@link String}. + * @param model type. The type of the Model class, that is wrapped by this ModelWrapper instance. + * @param return type. The type of the Property that is returned via {@link #getProperty()}, f.e. {@link StringProperty} or {@link Property}. */ private interface PropertyField> { void commit(M wrappedObject); @@ -232,6 +230,14 @@ private interface PropertyField> { R getProperty(); + /** + * Determines if the value in the model object and the property field are different or not. + * + * This method is used to implement the {@link #differentProperty()} flag. + * + * @param wrappedObject the wrapped model object + * @return false if both the wrapped model object and the property field have the same value, otherwise true + */ boolean isDifferent(M wrappedObject); } @@ -348,8 +354,8 @@ public boolean isDifferent(M wrappedObject) { } /** - * An implementation of {@link PropertyField} that is used when the field of the model class is a {@link List} and - * and is a JavaFX {@link ListProperty} too. + * An implementation of {@link PropertyField} that is used when the field of the model class is a {@link List} + * and will be mapped to a JavaFX {@link ListProperty}. * * @param * @param @@ -370,7 +376,7 @@ public FxListPropertyField(ListPropertyAccessor accessor, List defaultV this.accessor = accessor; this.defaultValue = defaultValue; - this.targetProperty.addListener((ListChangeListener) change -> propertyWasChanged()); + this.targetProperty.addListener((ListChangeListener) change -> ModelWrapper.this.propertyWasChanged()); } @Override @@ -398,7 +404,7 @@ public boolean isDifferent(M wrappedObject) { final List modelValue = accessor.apply(wrappedObject).getValue(); final List wrapperValue = targetProperty; - return !(modelValue.containsAll(wrapperValue) && wrapperValue.containsAll(modelValue)); + return !Objects.equals(modelValue, wrapperValue); } } @@ -428,8 +434,8 @@ public BeanListPropertyField(ListGetter getter, ListSetter setter, L this.defaultValue = defaultValue; this.getter = getter; this.setter = setter; - - this.targetProperty.addListener((ListChangeListener) change -> propertyWasChanged()); + + this.targetProperty.addListener((ListChangeListener) change -> propertyWasChanged()); } @Override @@ -457,7 +463,7 @@ public boolean isDifferent(M wrappedObject) { final List modelValue = getter.apply(wrappedObject); final List wrapperValue = targetProperty; - return !(modelValue.containsAll(wrapperValue) && wrapperValue.containsAll(modelValue)); + return !Objects.equals(modelValue, wrapperValue); } } diff --git a/mvvmfx/src/test/java/de/saxsys/mvvmfx/utils/mapping/ModelWrapperTest.java b/mvvmfx/src/test/java/de/saxsys/mvvmfx/utils/mapping/ModelWrapperTest.java index 45cadbef1..a88f00182 100644 --- a/mvvmfx/src/test/java/de/saxsys/mvvmfx/utils/mapping/ModelWrapperTest.java +++ b/mvvmfx/src/test/java/de/saxsys/mvvmfx/utils/mapping/ModelWrapperTest.java @@ -372,9 +372,6 @@ public void testDifferentFlag() { assertThat(personWrapper.isDifferent()).isFalse(); - nicknames.remove("captain"); - assertThat(personWrapper.isDifferent()).isTrue(); - nicknames.remove("captain"); assertThat(personWrapper.isDifferent()).isTrue(); @@ -393,8 +390,19 @@ public void testDifferentFlag() { personWrapper.reload(); assertThat(personWrapper.isDifferent()).isFalse(); - nicknames.add("captain"); - assertThat(personWrapper.isDifferent()).isFalse(); + nicknames.add("captain"); // duplicate captain + assertThat(personWrapper.isDifferent()).isTrue(); + + person.getNicknames().add("captain"); // now both have 2x "captain" but the modelWrapper has no chance to realize this change in the model element... + // ... for this reason the different flag will still be true + assertThat(personWrapper.isDifferent()).isTrue(); + + // ... but if we add another value to the nickname-Property, the modelWrapper can react to this change + person.getNicknames().add("other"); + nicknames.add("other"); + assertThat(personWrapper.isDifferent()).isFalse(); + + nicknames.add("player"); assertThat(personWrapper.isDifferent()).isTrue(); @@ -455,12 +463,23 @@ public void testDifferentFlagWithFxProperties() { nicknames.add("captain"); assertThat(personWrapper.isDifferent()).isFalse(); + + person.getNicknames().add("captain"); // duplicate value + nicknames.add("captain"); + assertThat(personWrapper.isDifferent()).isFalse(); nicknames.add("player"); assertThat(personWrapper.isDifferent()).isTrue(); - nicknames.remove("player"); + person.getNicknames().add("player"); + assertThat(personWrapper.isDifferent()).isTrue(); // still true because the modelWrapper can't detect the change in the model + + person.setName("luise"); + name.set("luise"); // this triggers the recalculation of the different-flag which will now detect the previous change to the nicknames list assertThat(personWrapper.isDifferent()).isFalse(); + + + nicknames.setValue(FXCollections.observableArrayList("spectator")); assertThat(personWrapper.isDifferent()).isTrue(); diff --git a/mvvmfx/src/test/java/de/saxsys/mvvmfx/utils/mapping/Person.java b/mvvmfx/src/test/java/de/saxsys/mvvmfx/utils/mapping/Person.java index f882a8afc..977f29cd9 100644 --- a/mvvmfx/src/test/java/de/saxsys/mvvmfx/utils/mapping/Person.java +++ b/mvvmfx/src/test/java/de/saxsys/mvvmfx/utils/mapping/Person.java @@ -1,8 +1,6 @@ package de.saxsys.mvvmfx.utils.mapping; -import javafx.collections.FXCollections; -import javafx.collections.ObservableList; - +import java.util.ArrayList; import java.util.List; public class Person { @@ -11,14 +9,14 @@ public class Person { private int age; - private ObservableList nicknames = FXCollections.observableArrayList(); + private List nicknames = new ArrayList<>(); public List getNicknames() { return nicknames; } public void setNicknames (List nicknames) { - this.nicknames.setAll(nicknames); + this.nicknames = nicknames; } public int getAge() {