From 11555413505dd8c6bf5e5cdbdcb1aead6059bacf Mon Sep 17 00:00:00 2001 From: kske Date: Wed, 22 Dec 2021 16:32:27 +0200 Subject: [PATCH 1/3] Fix javafx module artifact name --- javafx/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javafx/pom.xml b/javafx/pom.xml index a836baa..04f3b51 100644 --- a/javafx/pom.xml +++ b/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"> 4.0.0 - javafx + undo-redo-javafx Undo-Redo JavaFX Integration From 5d1ef847702ae1493e795129c1c5e8a539d4ff19 Mon Sep 17 00:00:00 2001 From: kske Date: Wed, 22 Dec 2021 16:51:33 +0200 Subject: [PATCH 2/3] Discard identity changes, discard divergent branches --- .../java/dev/kske/undoredo/core/Change.java | 6 ++-- .../dev/kske/undoredo/core/ChangeManager.java | 6 ++-- .../undoredo/core/UnlimitedChangeManager.java | 23 ++++++++++++--- .../kske/undoredo/core/ChangeManagerTest.java | 28 +++++++++++++++++++ .../undoredo/javafx/ChangeManagerWrapper.java | 9 ++++-- 5 files changed, 61 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/dev/kske/undoredo/core/Change.java b/core/src/main/java/dev/kske/undoredo/core/Change.java index 4252e03..b088c7a 100644 --- a/core/src/main/java/dev/kske/undoredo/core/Change.java +++ b/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 */ diff --git a/core/src/main/java/dev/kske/undoredo/core/ChangeManager.java b/core/src/main/java/dev/kske/undoredo/core/ChangeManager.java index 5410cfc..8b6c6ad 100644 --- a/core/src/main/java/dev/kske/undoredo/core/ChangeManager.java +++ b/core/src/main/java/dev/kske/undoredo/core/ChangeManager.java @@ -19,12 +19,14 @@ import java.util.*; public interface ChangeManager { /** - * 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 diff --git a/core/src/main/java/dev/kske/undoredo/core/UnlimitedChangeManager.java b/core/src/main/java/dev/kske/undoredo/core/UnlimitedChangeManager.java index 4b0d13b..d3da43e 100644 --- a/core/src/main/java/dev/kske/undoredo/core/UnlimitedChangeManager.java +++ b/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 the change type to store in this change manager * @author Maximilian Käfer * @author Kai S. K. Engelbart @@ -15,11 +17,24 @@ public final class UnlimitedChangeManager 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 + for (int i = index + 1; i < changes.size(); ++i) + changes.remove(i); + + changes.add(change); + ++index; + return true; + } + return false; } @Override diff --git a/core/src/test/java/dev/kske/undoredo/core/ChangeManagerTest.java b/core/src/test/java/dev/kske/undoredo/core/ChangeManagerTest.java index 51ac613..21ab08d 100644 --- a/core/src/test/java/dev/kske/undoredo/core/ChangeManagerTest.java +++ b/core/src/test/java/dev/kske/undoredo/core/ChangeManagerTest.java @@ -30,7 +30,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(new IntChange(wrapper, 0))); assertSame(1, wrapper.value); } @@ -100,6 +113,21 @@ 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.undo(); + assertTrue(manager.addChange(new IntChange(wrapper, 2))); + assertFalse(manager.isRedoAvailable()); + assertSame(2, wrapper.value); + } + /** * Tests marking a change. * diff --git a/javafx/src/main/java/dev/kske/undoredo/javafx/ChangeManagerWrapper.java b/javafx/src/main/java/dev/kske/undoredo/javafx/ChangeManagerWrapper.java index 0c16cb6..b059104 100644 --- a/javafx/src/main/java/dev/kske/undoredo/javafx/ChangeManagerWrapper.java +++ b/javafx/src/main/java/dev/kske/undoredo/javafx/ChangeManagerWrapper.java @@ -50,9 +50,12 @@ public class ChangeManagerWrapper> } @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 From d484839c8bd9cedf7bdb3982e81f5f548293c75e Mon Sep 17 00:00:00 2001 From: kske Date: Thu, 23 Dec 2021 09:42:33 +0200 Subject: [PATCH 3/3] Fix divergent changes removal, improve unit test --- .../undoredo/core/UnlimitedChangeManager.java | 3 +-- .../kske/undoredo/core/ChangeManagerTest.java | 22 +++++++++++++++---- .../dev/kske/undoredo/core/IntChange.java | 22 +++++++++++++++++++ .../dev/kske/undoredo/core/IntWrapper.java | 22 +++++++++++++++++++ 4 files changed, 63 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/dev/kske/undoredo/core/UnlimitedChangeManager.java b/core/src/main/java/dev/kske/undoredo/core/UnlimitedChangeManager.java index d3da43e..e62dd44 100644 --- a/core/src/main/java/dev/kske/undoredo/core/UnlimitedChangeManager.java +++ b/core/src/main/java/dev/kske/undoredo/core/UnlimitedChangeManager.java @@ -27,8 +27,7 @@ public final class UnlimitedChangeManager implements ChangeMan change.apply(); // Remove divergent changes - for (int i = index + 1; i < changes.size(); ++i) - changes.remove(i); + changes.subList(index + 1, changes.size()).clear(); changes.add(change); ++index; diff --git a/core/src/test/java/dev/kske/undoredo/core/ChangeManagerTest.java b/core/src/test/java/dev/kske/undoredo/core/ChangeManagerTest.java index 21ab08d..3f77620 100644 --- a/core/src/test/java/dev/kske/undoredo/core/ChangeManagerTest.java +++ b/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); } /** @@ -43,7 +54,7 @@ class ChangeManagerTest { @Order(15) void testDiscardIdentity() { manager.addChange(change); - assertFalse(manager.addChange(new IntChange(wrapper, 0))); + assertFalse(manager.addChange(change(0))); assertSame(1, wrapper.value); } @@ -122,10 +133,13 @@ class ChangeManagerTest { @Order(55) void testDiscardDivergentHistory() { manager.addChange(change); + manager.addChange(change(2)); manager.undo(); - assertTrue(manager.addChange(new IntChange(wrapper, 2))); + manager.undo(); + assertTrue(manager.addChange(change(3))); assertFalse(manager.isRedoAvailable()); - assertSame(2, wrapper.value); + assertSame(3, wrapper.value); + assertSame(1, manager.getChanges().size()); } /** diff --git a/core/src/test/java/dev/kske/undoredo/core/IntChange.java b/core/src/test/java/dev/kske/undoredo/core/IntChange.java index f6e8a32..d3fabe1 100644 --- a/core/src/test/java/dev/kske/undoredo/core/IntChange.java +++ b/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); + } } diff --git a/core/src/test/java/dev/kske/undoredo/core/IntWrapper.java b/core/src/test/java/dev/kske/undoredo/core/IntWrapper.java index 2b6907d..74a667d 100644 --- a/core/src/test/java/dev/kske/undoredo/core/IntWrapper.java +++ b/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; + } }