Correctly Deal with Identity Changes and Divergent History #5
@ -2,7 +2,7 @@ package dev.kske.undoredo.core;
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Base interface for changes to be registered in an undo manager.
|
* Base interface for changes to be registered in an undo manager.
|
||||||
*
|
*
|
||||||
* @author Maximilian Käfer
|
* @author Maximilian Käfer
|
||||||
* @since 0.0.1
|
* @since 0.0.1
|
||||||
*/
|
*/
|
||||||
@ -17,7 +17,7 @@ public interface Change {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Inverts this change.
|
* Inverts this change.
|
||||||
*
|
*
|
||||||
* @implSpec This method is not supposed to alter the state of this change, but rather to create
|
* @implSpec This method is not supposed to alter the state of this change, but rather to create
|
||||||
* a new complementary change.
|
* a new complementary change.
|
||||||
* @return the inverted change
|
* @return the inverted change
|
||||||
@ -26,6 +26,8 @@ public interface Change {
|
|||||||
Change invert();
|
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
|
* @return whether the application of this change would result in an identical state
|
||||||
* @since 0.0.1
|
* @since 0.0.1
|
||||||
*/
|
*/
|
||||||
|
@ -19,12 +19,14 @@ import java.util.*;
|
|||||||
public interface ChangeManager<C extends Change> {
|
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
|
* @param change the change to add
|
||||||
|
* @return whether the change has been added
|
||||||
* @since 0.0.1
|
* @since 0.0.1
|
||||||
*/
|
*/
|
||||||
void addChange(C change);
|
boolean addChange(C change);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @return the change that was applied last
|
* @return the change that was applied last
|
||||||
|
@ -3,6 +3,8 @@ package dev.kske.undoredo.core;
|
|||||||
import java.util.*;
|
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
|
* @param <C> the change type to store in this change manager
|
||||||
* @author Maximilian Käfer
|
* @author Maximilian Käfer
|
||||||
* @author Kai S. K. Engelbart
|
* @author Kai S. K. Engelbart
|
||||||
@ -15,11 +17,24 @@ public final class UnlimitedChangeManager<C extends Change> implements ChangeMan
|
|||||||
private int index = -1;
|
private int index = -1;
|
||||||
private int markedIndex = -1;
|
private int markedIndex = -1;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @implNode As this change manager uses a linear history model, all changes behind the last
|
||||||
delvh marked this conversation as resolved
|
|||||||
|
* applied change will be discarded and therefore can be garbage collected.
|
||||||
|
*/
|
||||||
@Override
|
@Override
|
||||||
public void addChange(C change) {
|
public boolean addChange(C change) {
|
||||||
change.apply();
|
if (!change.isIdentity()) {
|
||||||
changes.add(change);
|
change.apply();
|
||||||
++index;
|
|
||||||
|
// Remove divergent changes
|
||||||
|
for (int i = index + 1; i < changes.size(); ++i)
|
||||||
|
changes.remove(i);
|
||||||
|
|
||||||
|
changes.add(change);
|
||||||
|
++index;
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
@ -30,7 +30,20 @@ class ChangeManagerTest {
|
|||||||
@Order(10)
|
@Order(10)
|
||||||
void testAddChange() {
|
void testAddChange() {
|
||||||
assertSame(0, wrapper.value);
|
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);
|
manager.addChange(change);
|
||||||
|
assertFalse(manager.addChange(new IntChange(wrapper, 0)));
|
||||||
assertSame(1, wrapper.value);
|
assertSame(1, wrapper.value);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -100,6 +113,21 @@ class ChangeManagerTest {
|
|||||||
assertEquals(change, manager.getLastChange().get());
|
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());
|
||||||
kske marked this conversation as resolved
delvh
commented
Perhaps you should Perhaps you should
a) test by adding two changes that are undone
b) Add an assertSame for the size of changes stored
kske
commented
Good point. I actually discovered a bug through this. The fix is coming in the next commit. Good point. I actually discovered a bug through this. The fix is coming in the next commit.
|
|||||||
|
assertSame(2, wrapper.value);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Tests marking a change.
|
* Tests marking a change.
|
||||||
*
|
*
|
||||||
|
@ -50,9 +50,12 @@ public class ChangeManagerWrapper<C extends Change, M extends ChangeManager<C>>
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void addChange(C change) {
|
public boolean addChange(C change) {
|
||||||
manager.addChange(change);
|
if (manager.addChange(change)) {
|
||||||
updateProperties();
|
updateProperties();
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
Reference in New Issue
Block a user
I can definitely see needing this Javadoc more often.
What about implementing a superclass called
LinearHistoryChangeManager
that automatically supplies the Javadoc and general implementations of functions (undo
,redo
, andaddChange
given a new protected abstract functionremoveDivergentChanges
)?I think we need to create more change manager implementations before deciding what belongs in the superclass and what doesn't. In any case, this branch is probably the wrong place add this.