Merge pull request 'Correctly Deal with Identity Changes and Divergent History' (#5) from b/divergent-history into develop

Reviewed-on: https://git.kske.dev/zdm/undo-redo/pulls/5
Reviewed-by: delvh <leon@kske.dev>

core/src/main/java/dev/kske/undoredo/core/Change.java

@@ -2,7 +2,7 @@ package dev.kske.undoredo.core;
 
 /**
  * Base interface for changes to be registered in an undo manager.
- * 
+ *
  * @author Maximilian Käfer
  * @since 0.0.1
  */
@@ -17,7 +17,7 @@ public interface Change {
 
 	/**
 	 * Inverts this change.
-	 * 
+	 *
 	 * @implSpec This method is not supposed to alter the state of this change, but rather to create
 	 *           a new complementary change.
 	 * @return the inverted change
@@ -26,6 +26,8 @@ public interface Change {
 	Change invert();
 
 	/**
+	 * @apiNote If this method returns {@code true} when adding this change to a change manager, it
+	 *          will be discarded immediately and therefore can be garbage collected.
 	 * @return whether the application of this change would result in an identical state
 	 * @since 0.0.1
 	 */

core/src/main/java/dev/kske/undoredo/core/ChangeManager.java

@@ -19,12 +19,14 @@ import java.util.*;
 public interface ChangeManager<C extends Change> {
 
 	/**
-	 * Applies the given change and appends it to the change list.
+	 * Applies the given change and appends it to the change list. If the change is an identity, no
+	 * action is taken.
 	 *
 	 * @param change the change to add
+	 * @return whether the change has been added
 	 * @since 0.0.1
 	 */
-	void addChange(C change);
+	boolean addChange(C change);
 
 	/**
 	 * @return the change that was applied last

core/src/main/java/dev/kske/undoredo/core/UnlimitedChangeManager.java

@@ -3,6 +3,8 @@ package dev.kske.undoredo.core;
 import java.util.*;
 
 /**
+ * A simple change manager with a linear history model backed by an array list.
+ *
  * @param <C> the change type to store in this change manager
  * @author Maximilian K&auml;fer
  * @author Kai S. K. Engelbart
@@ -15,11 +17,23 @@ public final class UnlimitedChangeManager<C extends Change> implements ChangeMan
 	private int	index		= -1;
 	private int	markedIndex	= -1;
 
+	/**
+	 * @implNode As this change manager uses a linear history model, all changes behind the last
+	 *           applied change will be discarded and therefore can be garbage collected.
+	 */
 	@Override
-	public void addChange(C change) {
-		change.apply();
-		changes.add(change);
-		++index;
+	public boolean addChange(C change) {
+		if (!change.isIdentity()) {
+			change.apply();
+
+			// Remove divergent changes
+			changes.subList(index + 1, changes.size()).clear();
+
+			changes.add(change);
+			++index;
+			return true;
+		}
+		return false;
 	}
 
 	@Override

core/src/test/java/dev/kske/undoredo/core/ChangeManagerTest.java

@@ -14,11 +14,22 @@ class ChangeManagerTest {
 	IntWrapper					wrapper;
 	IntChange					change;
 
+	/**
+	 * Generates an int change with the given value.
+	 *
+	 * @param value the value of the change
+	 * @return the created change
+	 * @since 0.0.1
+	 */
+	IntChange change(int value) {
+		return new IntChange(wrapper, value);
+	}
+
 	@BeforeEach
 	void prepareChangeManager() {
 		manager	= new UnlimitedChangeManager<>();
 		wrapper	= new IntWrapper();
-		change	= new IntChange(wrapper, 1);
+		change	= change(1);
 	}
 
 	/**
@@ -30,7 +41,20 @@ class ChangeManagerTest {
 	@Order(10)
 	void testAddChange() {
 		assertSame(0, wrapper.value);
+		assertTrue(manager.addChange(change));
+		assertSame(1, wrapper.value);
+	}
+
+	/**
+	 * Tests whether identity changes are ignored when adding them to the change manager.
+	 *
+	 * @since 0.0.1
+	 */
+	@Test
+	@Order(15)
+	void testDiscardIdentity() {
 		manager.addChange(change);
+		assertFalse(manager.addChange(change(0)));
 		assertSame(1, wrapper.value);
 	}
 
@@ -100,6 +124,24 @@ class ChangeManagerTest {
 		assertEquals(change, manager.getLastChange().get());
 	}
 
+	/**
+	 * Tests whether the changes after the current index are discarded when adding a change.
+	 *
+	 * @since 0.0.1
+	 */
+	@Test
+	@Order(55)
+	void testDiscardDivergentHistory() {
+		manager.addChange(change);
+		manager.addChange(change(2));
+		manager.undo();
+		manager.undo();
+		assertTrue(manager.addChange(change(3)));
+		assertFalse(manager.isRedoAvailable());
+		assertSame(3, wrapper.value);
+		assertSame(1, manager.getChanges().size());
+	}
+
 	/**
 	 * Tests marking a change.
 	 *

core/src/test/java/dev/kske/undoredo/core/IntChange.java

@@ -1,5 +1,7 @@
 package dev.kske.undoredo.core;
 
+import java.util.Objects;
+
 /**
  * @author Kai S. K. Engelbart
  * @since 0.0.1
@@ -28,4 +30,24 @@ class IntChange implements Change {
 	public boolean isIdentity() {
 		return value == 0;
 	}
+
+	@Override
+	public String toString() {
+		return String.valueOf(value);
+	}
+
+	@Override
+	public int hashCode() {
+		return Objects.hash(value, wrapper);
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (this == obj)
+			return true;
+		if (!(obj instanceof IntChange))
+			return false;
+		IntChange other = (IntChange) obj;
+		return value == other.value && Objects.equals(wrapper, other.wrapper);
+	}
 }

core/src/test/java/dev/kske/undoredo/core/IntWrapper.java

@@ -1,5 +1,7 @@
 package dev.kske.undoredo.core;
 
+import java.util.Objects;
+
 /**
  * @author Kai S. K. Engelbart
  * @since 0.0.1
@@ -7,4 +9,24 @@ package dev.kske.undoredo.core;
 class IntWrapper {
 
 	int value;
+
+	@Override
+	public String toString() {
+		return String.valueOf(value);
+	}
+
+	@Override
+	public int hashCode() {
+		return Objects.hash(value);
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (this == obj)
+			return true;
+		if (!(obj instanceof IntWrapper))
+			return false;
+		IntWrapper other = (IntWrapper) obj;
+		return value == other.value;
+	}
 }

javafx/pom.xml

@@ -3,7 +3,7 @@
 	xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
 	<modelVersion>4.0.0</modelVersion>
 
-	<artifactId>javafx</artifactId>
+	<artifactId>undo-redo-javafx</artifactId>
 	<name>Undo-Redo JavaFX Integration</name>
 
 	<parent>

javafx/src/main/java/dev/kske/undoredo/javafx/ChangeManagerWrapper.java

@@ -50,9 +50,12 @@ public class ChangeManagerWrapper<C extends Change, M extends ChangeManager<C>>
 	}
 
 	@Override
-	public void addChange(C change) {
-		manager.addChange(change);
-		updateProperties();
+	public boolean addChange(C change) {
+		if (manager.addChange(change)) {
+			updateProperties();
+			return true;
+		}
+		return false;
 	}
 
 	@Override