Merge pull request #306 from sialcasa/pr/298

#298 some small fixes for the pull request for list mapping

Author: manuel-mauky

mvvmfx/src/main/java/de/saxsys/mvvmfx/utils/mapping/ModelWrapper.java

@@ -204,9 +204,6 @@
  * 
  * 
  * 
- * 
- * 
- * 
  * @param <M>
  *            the type of the model class.
  */
@@ -220,8 +217,9 @@ public class ModelWrapper<M> {
 	/**
 	 * This interface defines the operations that are possible for each field of a wrapped class.
 	 * 
-	 * @param <T>
-	 * @param <M>
+	 * @param <T> target type. The base type of the returned property, f.e. {@link String}.
+	 * @param <M> model type. The type of the Model class, that is wrapped by this ModelWrapper instance.
+	 * @param <R> return type. The type of the Property that is returned via {@link #getProperty()}, f.e. {@link StringProperty} or {@link Property<String>}. 
 	 */
 	private interface PropertyField<T, M, R extends Property<T>> {
 		void commit(M wrappedObject);
@@ -232,6 +230,14 @@ private interface PropertyField<T, M, R extends Property<T>> {
 
 		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 <code>false</code> if both the wrapped model object and the property field have the same value, otherwise <code>true</code>
+		 */
         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 <T>
 	 * @param <E>
@@ -370,7 +376,7 @@ public FxListPropertyField(ListPropertyAccessor<M, E> accessor, List<E> defaultV
 			this.accessor = accessor;
 			this.defaultValue = defaultValue;
 
-			this.targetProperty.addListener((ListChangeListener) change -> propertyWasChanged());
+			this.targetProperty.addListener((ListChangeListener<E>) change -> ModelWrapper.this.propertyWasChanged());
 		}
 
 		@Override
@@ -398,7 +404,7 @@ public boolean isDifferent(M wrappedObject) {
 			final List<E> modelValue = accessor.apply(wrappedObject).getValue();
 			final List<E> wrapperValue = targetProperty;
 
-			return !(modelValue.containsAll(wrapperValue) && wrapperValue.containsAll(modelValue));
+			return !Objects.equals(modelValue, wrapperValue);
 		}
 	}
 
@@ -428,8 +434,8 @@ public BeanListPropertyField(ListGetter<M, E> getter, ListSetter<M, E> setter, L
 			this.defaultValue = defaultValue;
 			this.getter = getter;
 			this.setter = setter;
-
-			this.targetProperty.addListener((ListChangeListener) change -> propertyWasChanged());
+			
+			this.targetProperty.addListener((ListChangeListener<E>) change -> propertyWasChanged());
 		}
 
 		@Override
@@ -457,7 +463,7 @@ public boolean isDifferent(M wrappedObject) {
 			final List<E> modelValue = getter.apply(wrappedObject);
 			final List<E> wrapperValue = targetProperty;
 
-			return !(modelValue.containsAll(wrapperValue) && wrapperValue.containsAll(modelValue));
+			return !Objects.equals(modelValue, wrapperValue);
 		}
 	}
 

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();

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<String> nicknames = FXCollections.observableArrayList();
+	private List<String> nicknames = new ArrayList<>();
 
 	public List<String> getNicknames() {
 		return nicknames;
 	}
 
 	public void setNicknames (List<String> nicknames) {
-		this.nicknames.setAll(nicknames);
+		this.nicknames = nicknames;
 	}
 
 	public int getAge() {