Skip to content

Commit

Permalink
fix: Fix issue where replace does not do a in-place replacement
Browse files Browse the repository at this point in the history
Bug was that newElement is always added to the back of the collection regardless of the position of existingElement.

Add unit test to check ordering after replace
  • Loading branch information
lhy-hoyin committed Oct 19, 2024
1 parent 0c15467 commit 57e4268
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import io.jsonwebtoken.lang.Strings;

import java.util.Collection;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.NoSuchElementException;

Expand Down Expand Up @@ -62,10 +63,25 @@ public M replace(E existingElement, E newElement) {
throw new NoSuchElementException(msg);
}

// FIXME: Ordering is not correct
if (this.collection.remove(existingElement))
if (doAdd(newElement))
changed(); // removed and add successfully, notify changed()
// Replacement step 1: iterate until element to replace
Iterator<E> it = this.collection.iterator();
while (it.hasNext())
if (it.next().equals(existingElement)) {
it.remove(); // step 2: remove existingElement
break;
}

// Replacement step 3: collect and remove elements after element to replace
Collection<E> elementsAfterExisting = new LinkedHashSet<>();
while (it.hasNext()) {
elementsAfterExisting.add(it.next());
it.remove();
}

this.doAdd(newElement); // step 4: add replacer element (position will be at the existingElement)
this.collection.addAll(elementsAfterExisting); // step 5: add back the elemnts found after existingElement

changed(); // trigger changed()

return self();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,13 @@ class DefaultCollectionMutatorTest {
m.replace('foo', Strings.EMPTY)
}

@Test
void replaceMaintainsOrder() {
m.add(['1', '2', '3'])
m.replace('2', 'B')
assertArrayEquals(new Object[] {'1', 'B', '3'}, m.collection.toArray())
}

@Test
void clear() {
m.add('one').add('two').add(['three', 'four'])
Expand Down

0 comments on commit 57e4268

Please sign in to comment.