From 7032769bc094b009b90ca33b54273f7ba64829b5 Mon Sep 17 00:00:00 2001 From: MonsterDruide1 <5958456@gmail.com> Date: Wed, 10 Aug 2022 14:44:49 +0200 Subject: [PATCH] gui/util/BidirectionalMap: Cleanup, fix, tests One notable change is the adjustment of functionality in addAll. Before, it was possible to create a map with duplicate keys/values by adding them twice using the same `addAll` call, as there were no check within that function call whether they are even valid in that sense. Apart from that, a few tests have been added to get the coverage to 100%. --- .../tools/aqua/bgw/util/BidirectionalMap.kt | 39 +++++++++++-------- .../aqua/bgw/util/bidirectionalmap/AddTest.kt | 39 +++++++++++++++++-- 2 files changed, 59 insertions(+), 19 deletions(-) diff --git a/bgw-gui/src/main/kotlin/tools/aqua/bgw/util/BidirectionalMap.kt b/bgw-gui/src/main/kotlin/tools/aqua/bgw/util/BidirectionalMap.kt index e76906fa39..56d75ab48a 100644 --- a/bgw-gui/src/main/kotlin/tools/aqua/bgw/util/BidirectionalMap.kt +++ b/bgw-gui/src/main/kotlin/tools/aqua/bgw/util/BidirectionalMap.kt @@ -54,6 +54,7 @@ open class BidirectionalMap(vararg elements: Pair) { private val map: MutableList> = mutableListOf() init { + // requires to add them one-by-one to throw an IllegalArgumentException for duplicates require(elements.all { add(it) }) } @@ -70,13 +71,7 @@ open class BidirectionalMap(vararg elements: Pair) { * * @see addAll */ - fun add(entity: T, value: R): Boolean { - if (contains(entity, value)) return true - - if (containsForward(entity) || containsBackward(value)) return false - - return map.add(Pair(entity, value)) - } + fun add(entity: T, value: R): Boolean = add(Pair(entity, value)) /** * Adds a relation A -> B if domain does not contain A and coDomain does not contain B. Returns @@ -86,11 +81,19 @@ open class BidirectionalMap(vararg elements: Pair) { * * @see addAll */ - fun add(element: Pair): Boolean = add(element.first, element.second) + fun add(element: Pair): Boolean { + if (contains(element)) return true + + if (containsForward(element.first) || containsBackward(element.second)) return false + + return map.add(element) + } /** * Adds all relations A -> B. If any of the given items already exist, it gets ignored. If any - * item contains a key or value that already exists, the map remains unchanged. + * item occurs twice in the list, one gets ignored. If any key or value is already in the map with + * a different mapping, or occurs twice with a different mapping, false is returned and the map + * remains unchanged. * * Example: Map: [(A->B), (C->D)] * @@ -106,11 +109,16 @@ open class BidirectionalMap(vararg elements: Pair) { * @see add */ fun addAll(vararg items: Pair): Boolean { - val nonDuplicates = items.filter { !contains(it) }.toList() + val nonDuplicates = items.distinct().filter { !contains(it) } if (nonDuplicates.any { containsForward(it.first) || containsBackward(it.second) }) return false + if (nonDuplicates.any { item -> + nonDuplicates.filter { it != item }.any { it.first == item.first } || + nonDuplicates.filter { it != item }.any { it.second == item.second } + }) + return false - nonDuplicates.forEach { map.add(it) } + map.addAll(nonDuplicates) return true } @@ -166,8 +174,7 @@ open class BidirectionalMap(vararg elements: Pair) { * @see removeForward * @see removeBackward */ - fun remove(entity: T, value: R): Boolean = - map.removeIf { t -> t.first == entity && t.second == value } + fun remove(entity: T, value: R): Boolean = remove(Pair(entity, value)) /** * Removes relation A -> B if it exists. @@ -179,7 +186,7 @@ open class BidirectionalMap(vararg elements: Pair) { * @see removeForward * @see removeBackward */ - fun remove(element: Pair): Boolean = remove(element.first, element.second) + fun remove(element: Pair): Boolean = map.remove(element) /** * Removes by forward lookup. Removes relation A -> * if it exists. @@ -216,7 +223,7 @@ open class BidirectionalMap(vararg elements: Pair) { * @see containsForward * @see containsBackward */ - fun contains(entity: T, value: R): Boolean = containsForward(entity) && containsBackward(value) + fun contains(entity: T, value: R): Boolean = contains(Pair(entity, value)) /** * Returns whether relation A -> B exists in this map. @@ -228,7 +235,7 @@ open class BidirectionalMap(vararg elements: Pair) { * @see containsForward * @see containsBackward */ - fun contains(pair: Pair): Boolean = contains(pair.first, pair.second) + fun contains(pair: Pair): Boolean = map.contains(pair) /** * Returns whether a relation A -> * exists. diff --git a/bgw-gui/src/test/kotlin/tools/aqua/bgw/util/bidirectionalmap/AddTest.kt b/bgw-gui/src/test/kotlin/tools/aqua/bgw/util/bidirectionalmap/AddTest.kt index 552fb329ca..797b6ed098 100644 --- a/bgw-gui/src/test/kotlin/tools/aqua/bgw/util/bidirectionalmap/AddTest.kt +++ b/bgw-gui/src/test/kotlin/tools/aqua/bgw/util/bidirectionalmap/AddTest.kt @@ -95,10 +95,10 @@ class AddTest : BidirectionalMapTestBase() { assertTrue(map.contains(5, 6)) } - /** Test adding new, old and invalid values by addAll. */ + /** Test adding new, old and invalid (by key) values by addAll. */ @Test - @DisplayName("Test adding new, old and invalid values by addAll") - fun testAddAllMixedWithInvalid() { + @DisplayName("Test adding new, old and invalid (by key) values by addAll") + fun testAddAllMixedWithInvalidKey() { assertFalse(map.addAll(Pair(5, 6), Pair(0, 5), Pair(0, 1))) assertEquals(2, map.size) @@ -106,6 +106,39 @@ class AddTest : BidirectionalMapTestBase() { assertFalse(map.contains(0, 5)) } + /** Test adding new, old and invalid (by value) values by addAll. */ + @Test + @DisplayName("Test adding new, old and invalid (by value) values by addAll") + fun testAddAllMixedWithInvalidValue() { + assertFalse(map.addAll(Pair(5, 6), Pair(7, 1), Pair(0, 1))) + + assertEquals(2, map.size) + assertFalse(map.contains(5, 6)) + assertFalse(map.contains(0, 5)) + } + + /** Test adding invalid (by key) values not contained before by addAll. */ + @Test + @DisplayName("Test adding invalid (by key) values not contained before by addAll") + fun testAddAllMultipleInvalidKey() { + assertFalse(map.addAll(Pair(7, 8), Pair(7, 9))) + + assertEquals(2, map.size) + assertFalse(map.contains(5, 6)) + assertFalse(map.contains(0, 5)) + } + + /** Test adding invalid (by value) values not contained before by addAll. */ + @Test + @DisplayName("Test adding invalid (by value) values not contained before by addAll") + fun testAddAllMultipleInvalidValue() { + assertFalse(map.addAll(Pair(7, 9), Pair(8, 9))) + + assertEquals(2, map.size) + assertFalse(map.contains(5, 6)) + assertFalse(map.contains(0, 5)) + } + /** Test adding old values by addAll. */ @Test @DisplayName("Test adding old values by addAll")