From 7eb4f1fb33546294420df6dbdabfac3fc9234851 Mon Sep 17 00:00:00 2001 From: Andras Slemmer Date: Wed, 31 Aug 2016 10:36:01 +0100 Subject: [PATCH] client: Clarify keyHashCode var name, add unit tests for AggregatedList --- .../r3corda/client/fxutils/AggregatedList.kt | 19 +++-- .../client/fxutils/AggregatedListTest.kt | 82 +++++++++++++++++++ 2 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 client/src/test/kotlin/com/r3corda/client/fxutils/AggregatedListTest.kt diff --git a/client/src/main/kotlin/com/r3corda/client/fxutils/AggregatedList.kt b/client/src/main/kotlin/com/r3corda/client/fxutils/AggregatedList.kt index 26084a285b..6d2fc6e7c8 100644 --- a/client/src/main/kotlin/com/r3corda/client/fxutils/AggregatedList.kt +++ b/client/src/main/kotlin/com/r3corda/client/fxutils/AggregatedList.kt @@ -7,10 +7,10 @@ import javafx.collections.transformation.TransformationList import kotlin.comparisons.compareValues /** - * Given an [ObservableList]<[E]>s and a grouping key [K], [AggregatedList] groups the elements by the key into a fresh - * [ObservableList] for each group and exposes the groups as an observable list of [A]s by calling [assemble] on each group. + * Given an [ObservableList]<[E]> and a grouping key [K], [AggregatedList] groups the elements by the key into a fresh + * [ObservableList]<[E]> for each group and exposes the groups as an observable list of [A]s by calling [assemble] on each. * - * Changes done to elements of the input list are reflected in the observable list of the respective group, whereas + * Changes done to elements of the input list are reflected in the observable list of the respective group, whereas * additions/removals of elements in the underlying list are reflected in the exposed [ObservableList]<[A]> by * adding/deleting aggregations as expected. * @@ -26,6 +26,9 @@ import kotlin.comparisons.compareValues * * The above creates an observable list of (currency, statesOfCurrency) pairs. * + * Note that update events to the source list are discarded, assuming the key of elements does not change. + * TODO Should we handle this case? It requires additional bookkeeping of sourceIndex->(aggregationIndex, groupIndex) + * * @param list The underlying list. * @param toKey Function to extract the key from an element. * @param assemble Function to assemble the aggregation into the exposed [A]. @@ -37,12 +40,12 @@ class AggregatedList( ) : TransformationList(list) { private class AggregationGroup( - val key: Int, + val keyHashCode: Int, val value: A, val elements: ObservableList ) - // Invariant: sorted by K + // Invariant: sorted by K.hashCode() private val aggregationList = mutableListOf>() init { @@ -90,7 +93,7 @@ class AggregatedList( val keyHashCode = key.hashCode() val index = aggregationList.binarySearch( - comparison = { group -> compareValues(keyHashCode, group.key.hashCode()) } + comparison = { group -> compareValues(keyHashCode, group.keyHashCode.hashCode()) } ) if (index < 0) { throw IllegalStateException("Removed element $removedItem does not map to an existing aggregation") @@ -108,14 +111,14 @@ class AggregatedList( val key = toKey(addedItem) val keyHashCode = key.hashCode() val index = aggregationList.binarySearch( - comparison = { group -> compareValues(keyHashCode, group.key.hashCode()) } + comparison = { group -> compareValues(keyHashCode, group.keyHashCode.hashCode()) } ) if (index < 0) { // New aggregation val observableGroupElements = FXCollections.observableArrayList() observableGroupElements.add(addedItem) val aggregationGroup = AggregationGroup( - key = keyHashCode, + keyHashCode = keyHashCode, value = assemble(key, observableGroupElements), elements = observableGroupElements ) diff --git a/client/src/test/kotlin/com/r3corda/client/fxutils/AggregatedListTest.kt b/client/src/test/kotlin/com/r3corda/client/fxutils/AggregatedListTest.kt new file mode 100644 index 0000000000..3a65ae920e --- /dev/null +++ b/client/src/test/kotlin/com/r3corda/client/fxutils/AggregatedListTest.kt @@ -0,0 +1,82 @@ +package com.r3corda.client.fxutils + +import javafx.collections.FXCollections +import org.junit.Before +import org.junit.Test +import kotlin.test.fail + +class AggregatedListTest { + + var sourceList = FXCollections.observableArrayList() + + @Before + fun setup() { + sourceList = FXCollections.observableArrayList() + } + + @Test + fun addWorks() { + val aggregatedList = AggregatedList(sourceList, { it % 3 }) { mod3, group -> Pair(mod3, group) } + require(aggregatedList.size == 0) { "Aggregation is empty is source list is" } + + sourceList.add(9) + require(aggregatedList.size == 1) { "Aggregation list has one element if one was added to source list" } + require(aggregatedList[0]!!.first == 0) + + sourceList.add(8) + require(aggregatedList.size == 2) { "Aggregation list has two elements if two were added to source list with different keys" } + + sourceList.add(6) + require(aggregatedList.size == 2) { "Aggregation list's size doesn't change if element with existing key is added" } + + aggregatedList.forEach { + when (it.first) { + 0 -> require(it.second.toSet() == setOf(6, 9)) + 2 -> require(it.second.size == 1) + else -> fail("No aggregation expected with key ${it.first}") + } + } + } + + @Test + fun removeWorks() { + val aggregatedList = AggregatedList(sourceList, { it % 3 }) { mod3, group -> Pair(mod3, group) } + sourceList.addAll(0, 1, 2, 3, 4) + + require(aggregatedList.size == 3) + aggregatedList.forEach { + when (it.first) { + 0 -> require(it.second.toSet() == setOf(0, 3)) + 1 -> require(it.second.toSet() == setOf(1, 4)) + 2 -> require(it.second.toSet() == setOf(2)) + else -> fail("No aggregation expected with key ${it.first}") + } + } + + sourceList.remove(4) + require(aggregatedList.size == 3) + aggregatedList.forEach { + when (it.first) { + 0 -> require(it.second.toSet() == setOf(0, 3)) + 1 -> require(it.second.toSet() == setOf(1)) + 2 -> require(it.second.toSet() == setOf(2)) + else -> fail("No aggregation expected with key ${it.first}") + } + } + + sourceList.remove(2, 4) + require(aggregatedList.size == 2) + aggregatedList.forEach { + when (it.first) { + 0 -> require(it.second.toSet() == setOf(0)) + 1 -> require(it.second.toSet() == setOf(1)) + else -> fail("No aggregation expected with key ${it.first}") + } + } + + sourceList.removeAll(0, 1) + require(aggregatedList.size == 0) + } +} + +