refactor: remove resource list indexing as it wasn't used

refinedmods · Nov 4, 2023 · 3e6d61c · 3e6d61c
1 parent e30a442
commit 3e6d61c
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 126 deletions.
diff --git a/...ain/java/com/refinedmods/refinedstorage2/api/resource/list/AbstractProxyResourceList.java b/...ain/java/com/refinedmods/refinedstorage2/api/resource/list/AbstractProxyResourceList.java
@@ -4,7 +4,6 @@
 
 import java.util.Collection;
 import java.util.Optional;
-import java.util.UUID;
 
 import org.apiguardian.api.API;
 
@@ -36,11 +35,6 @@ public Optional<ResourceAmount<T>> get(final T resource) {
         return delegate.get(resource);
     }
 
-    @Override
-    public Optional<ResourceAmount<T>> get(final UUID id) {
-        return delegate.get(id);
-    }
-
     @Override
     public Collection<ResourceAmount<T>> getAll() {
         return delegate.getAll();

diff --git a/...rce-api/src/main/java/com/refinedmods/refinedstorage2/api/resource/list/ResourceList.java b/...rce-api/src/main/java/com/refinedmods/refinedstorage2/api/resource/list/ResourceList.java
@@ -4,7 +4,6 @@
 
 import java.util.Collection;
 import java.util.Optional;
-import java.util.UUID;
 
 import org.apiguardian.api.API;
 
@@ -66,14 +65,6 @@ default Optional<ResourceListOperationResult<T>> remove(ResourceAmount<T> resour
      */
     Optional<ResourceAmount<T>> get(T resource);
 
-    /**
-     * Retrieves the resource and its amount from the list, identified by ID.
-     *
-     * @param id the id
-     * @return the resource amount if it's present in the list, otherwise an empty {@link Optional}
-     */
-    Optional<ResourceAmount<T>> get(UUID id);
-
     /**
      * Retrieves all resources and their amounts from the list.
      *

diff --git a/...api/src/main/java/com/refinedmods/refinedstorage2/api/resource/list/ResourceListImpl.java b/...api/src/main/java/com/refinedmods/refinedstorage2/api/resource/list/ResourceListImpl.java
@@ -6,7 +6,6 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
-import java.util.UUID;
 
 import org.apiguardian.api.API;
 
@@ -19,8 +18,6 @@
 @API(status = API.Status.STABLE, since = "2.0.0-milestone.1.2")
 public class ResourceListImpl<T> implements ResourceList<T> {
     private final Map<T, ResourceAmount<T>> entries = new HashMap<>();
-    private final Map<UUID, ResourceAmount<T>> index = new HashMap<>();
-    private final Map<ResourceAmount<T>, UUID> inverseIndex = new HashMap<>();
 
     @Override
     public ResourceListOperationResult<T> add(final T resource, final long amount) {
@@ -35,19 +32,13 @@ public ResourceListOperationResult<T> add(final T resource, final long amount) {
     private ResourceListOperationResult<T> addToExisting(final ResourceAmount<T> resourceAmount, final long amount) {
         resourceAmount.increment(amount);
 
-        return new ResourceListOperationResult<>(resourceAmount, amount, inverseIndex.get(resourceAmount), true);
+        return new ResourceListOperationResult<>(resourceAmount, amount, true);
     }
 
     private ResourceListOperationResult<T> addNew(final T resource, final long amount) {
         final ResourceAmount<T> resourceAmount = new ResourceAmount<>(resource, amount);
-
-        final UUID id = UUID.randomUUID();
-
-        index.put(id, resourceAmount);
-        inverseIndex.put(resourceAmount, id);
         entries.put(resource, resourceAmount);
-
-        return new ResourceListOperationResult<>(resourceAmount, amount, id, true);
+        return new ResourceListOperationResult<>(resourceAmount, amount, true);
     }
 
     @Override
@@ -56,36 +47,29 @@ public Optional<ResourceListOperationResult<T>> remove(final T resource, final l
 
         final ResourceAmount<T> existing = entries.get(resource);
         if (existing != null) {
-            final UUID id = inverseIndex.get(existing);
-
             if (existing.getAmount() - amount <= 0) {
-                return removeCompletely(existing, id);
+                return removeCompletely(existing);
             } else {
-                return removePartly(amount, existing, id);
+                return removePartly(amount, existing);
             }
         }
 
         return Optional.empty();
     }
 
     private Optional<ResourceListOperationResult<T>> removePartly(final long amount,
-                                                                  final ResourceAmount<T> resourceAmount,
-                                                                  final UUID id) {
+                                                                  final ResourceAmount<T> resourceAmount) {
         resourceAmount.decrement(amount);
 
-        return Optional.of(new ResourceListOperationResult<>(resourceAmount, -amount, id, true));
+        return Optional.of(new ResourceListOperationResult<>(resourceAmount, -amount, true));
     }
 
-    private Optional<ResourceListOperationResult<T>> removeCompletely(final ResourceAmount<T> resourceAmount,
-                                                                      final UUID id) {
-        index.remove(id);
-        inverseIndex.remove(resourceAmount);
+    private Optional<ResourceListOperationResult<T>> removeCompletely(final ResourceAmount<T> resourceAmount) {
         entries.remove(resourceAmount.getResource());
 
         return Optional.of(new ResourceListOperationResult<>(
             resourceAmount,
             -resourceAmount.getAmount(),
-            id,
             false
         ));
     }
@@ -95,19 +79,13 @@ public Optional<ResourceAmount<T>> get(final T resource) {
         return Optional.ofNullable(entries.get(resource));
     }
 
-    @Override
-    public Optional<ResourceAmount<T>> get(final UUID id) {
-        return Optional.ofNullable(index.get(id));
-    }
-
     @Override
     public Collection<ResourceAmount<T>> getAll() {
         return entries.values();
     }
 
     @Override
     public void clear() {
-        index.clear();
         entries.clear();
     }
 }
diff --git a/...n/java/com/refinedmods/refinedstorage2/api/resource/list/ResourceListOperationResult.java b/...n/java/com/refinedmods/refinedstorage2/api/resource/list/ResourceListOperationResult.java
@@ -2,8 +2,6 @@
 
 import com.refinedmods.refinedstorage2.api.resource.ResourceAmount;
 
-import java.util.UUID;
-
 import org.apiguardian.api.API;
 
 /**
@@ -12,12 +10,8 @@
  * @param <T>            the type of resource
  * @param resourceAmount the current resource amount in the list
  * @param change         the delta caused by the operation
- * @param id             the id of the resource in the list
  * @param available      whether this resource is still available in the list, or if it was removed
  */
 @API(status = API.Status.STABLE, since = "2.0.0-milestone.1.2")
-public record ResourceListOperationResult<T>(ResourceAmount<T> resourceAmount,
-                                             long change,
-                                             UUID id,
-                                             boolean available) {
+public record ResourceListOperationResult<T>(ResourceAmount<T> resourceAmount, long change, boolean available) {
 }
diff --git a/...test/java/com/refinedmods/refinedstorage2/api/resource/list/AbstractResourceListTest.java b/...test/java/com/refinedmods/refinedstorage2/api/resource/list/AbstractResourceListTest.java
@@ -5,7 +5,6 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Optional;
-import java.util.UUID;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -30,7 +29,6 @@ void shouldAddNewResource() {
         final ResourceListOperationResult<String> result = list.add("A", 10);
 
         // Assert
-        assertThat(result.id()).isNotNull();
         assertThat(result.change()).isEqualTo(10);
         assertThat(result.resourceAmount().getAmount()).isEqualTo(10);
         assertThat(result.resourceAmount().getResource()).isEqualTo("A");
@@ -47,7 +45,6 @@ void shouldAddNewResourceWithResourceAmountDirectly() {
         final ResourceListOperationResult<String> result = list.add(new ResourceAmount<>("A", 10));
 
         // Assert
-        assertThat(result.id()).isNotNull();
         assertThat(result.change()).isEqualTo(10);
         assertThat(result.resourceAmount().getAmount()).isEqualTo(10);
         assertThat(result.resourceAmount().getResource()).isEqualTo("A");
@@ -65,13 +62,11 @@ void shouldAddMultipleOfSameResource() {
         final ResourceListOperationResult<String> result2 = list.add("A", 5);
 
         // Assert
-        assertThat(result1.id()).isNotNull();
         assertThat(result1.change()).isEqualTo(10);
         assertThat(result1.resourceAmount().getAmount()).isEqualTo(15);
         assertThat(result1.resourceAmount().getResource()).isEqualTo("A");
         assertThat(result1.available()).isTrue();
 
-        assertThat(result2.id()).isEqualTo(result1.id());
         assertThat(result2.change()).isEqualTo(5);
         assertThat(result1.resourceAmount().getAmount()).isEqualTo(15);
         assertThat(result1.resourceAmount().getResource()).isEqualTo("A");
@@ -90,19 +85,16 @@ void shouldAddMultipleOfDifferentResources() {
         final ResourceListOperationResult<String> result3 = list.add("B", 3);
 
         // Assert
-        assertThat(result1.id()).isNotNull();
         assertThat(result1.change()).isEqualTo(10);
         assertThat(result1.resourceAmount().getAmount()).isEqualTo(15);
         assertThat(result1.resourceAmount().getResource()).isEqualTo("A");
         assertThat(result1.available()).isTrue();
 
-        assertThat(result2.id()).isEqualTo(result1.id());
         assertThat(result2.change()).isEqualTo(5);
         assertThat(result2.resourceAmount().getAmount()).isEqualTo(15);
         assertThat(result2.resourceAmount().getResource()).isEqualTo("A");
         assertThat(result2.available()).isTrue();
 
-        assertThat(result3.id()).isEqualTo(result3.id());
         assertThat(result3.change()).isEqualTo(3);
         assertThat(result3.resourceAmount().getAmount()).isEqualTo(3);
         assertThat(result3.resourceAmount().getResource()).isEqualTo("B");
@@ -140,15 +132,14 @@ void shouldNotRemoveResourceWhenItIsNotAvailable() {
     @Test
     void shouldRemoveResourcePartly() {
         // Arrange
-        final ResourceListOperationResult<String> result1 = list.add("A", 20);
+        list.add("A", 20);
         list.add("B", 6);
 
         // Act
         final Optional<ResourceListOperationResult<String>> result2 = list.remove("A", 5);
 
         // Assert
         assertThat(result2).isPresent();
-        assertThat(result2.get().id()).isEqualTo(result1.id());
         assertThat(result2.get().change()).isEqualTo(-5);
         assertThat(result2.get().resourceAmount().getAmount()).isEqualTo(15);
         assertThat(result2.get().resourceAmount().getResource()).isEqualTo("A");
@@ -163,7 +154,7 @@ void shouldRemoveResourcePartly() {
     @Test
     void shouldRemoveResourcePartlyWithResourceAmountDirectly() {
         // Arrange
-        final ResourceListOperationResult<String> result1 = list.add("A", 20);
+        list.add("A", 20);
         list.add("B", 6);
 
         // Act
@@ -174,7 +165,6 @@ void shouldRemoveResourcePartlyWithResourceAmountDirectly() {
 
         // Assert
         assertThat(result2).isPresent();
-        assertThat(result2.get().id()).isEqualTo(result1.id());
         assertThat(result2.get().change()).isEqualTo(-5);
         assertThat(result2.get().resourceAmount().getAmount()).isEqualTo(15);
         assertThat(result2.get().resourceAmount().getResource()).isEqualTo("A");
@@ -189,15 +179,14 @@ void shouldRemoveResourcePartlyWithResourceAmountDirectly() {
     @Test
     void shouldRemoveResourceCompletely() {
         // Arrange
-        final ResourceListOperationResult<String> result1 = list.add("A", 20);
+        list.add("A", 20);
         list.add("B", 6);
 
         // Act
         final Optional<ResourceListOperationResult<String>> result2 = list.remove("A", 20);
 
         // Assert
         assertThat(result2).isPresent();
-        assertThat(result2.get().id()).isEqualTo(result1.id());
         assertThat(result2.get().change()).isEqualTo(-20);
         assertThat(result2.get().resourceAmount().getAmount()).isEqualTo(20);
         assertThat(result2.get().resourceAmount().getResource()).isEqualTo("A");
@@ -211,7 +200,7 @@ void shouldRemoveResourceCompletely() {
     @Test
     void shouldRemoveResourceCompletelyWithResourceAmountDirectly() {
         // Arrange
-        final ResourceListOperationResult<String> result1 = list.add("A", 20);
+        list.add("A", 20);
         list.add("B", 6);
 
         // Act
@@ -222,7 +211,6 @@ void shouldRemoveResourceCompletelyWithResourceAmountDirectly() {
 
         // Assert
         assertThat(result2).isPresent();
-        assertThat(result2.get().id()).isEqualTo(result1.id());
         assertThat(result2.get().change()).isEqualTo(-20);
         assertThat(result2.get().resourceAmount().getAmount()).isEqualTo(20);
         assertThat(result2.get().resourceAmount().getResource()).isEqualTo("A");
@@ -236,15 +224,14 @@ void shouldRemoveResourceCompletelyWithResourceAmountDirectly() {
     @Test
     void shouldNotRemoveResourceWithMoreThanIsAvailable() {
         // Arrange
-        final ResourceListOperationResult<String> result1 = list.add("A", 20);
+        list.add("A", 20);
         list.add("B", 6);
 
         // Act
         final Optional<ResourceListOperationResult<String>> result2 = list.remove("A", 21);
 
         // Assert
         assertThat(result2).isPresent();
-        assertThat(result2.get().id()).isEqualTo(result1.id());
         assertThat(result2.get().change()).isEqualTo(-20);
         assertThat(result2.get().resourceAmount().getAmount()).isEqualTo(20);
         assertThat(result2.get().resourceAmount().getResource()).isEqualTo("A");
@@ -283,35 +270,6 @@ void shouldBeAbleToRetrieveByResourceAfterAdding() {
         assertThat(resourceAmount.get().getAmount()).isEqualTo(6);
     }
 
-    @Test
-    void shouldBeAbleToRetrieveByIdAfterAdding() {
-        // Arrange
-        final ResourceListOperationResult<String> result = list.add("A", 3);
-
-        // Act
-        final Optional<ResourceAmount<String>> resourceAmount = list.get(result.id());
-
-        // Assert
-        assertThat(resourceAmount).isPresent();
-        assertThat(resourceAmount.get().getResource()).isEqualTo("A");
-        assertThat(resourceAmount.get().getAmount()).isEqualTo(3);
-    }
-
-    @Test
-    void shouldStillBeAbleToRetrieveByIdWhenRemovingPartly() {
-        // Arrange
-        final ResourceListOperationResult<String> result = list.add("A", 10);
-        list.remove("A", 3);
-
-        // Act
-        final Optional<ResourceAmount<String>> resourceAmount = list.get(result.id());
-
-        // Assert
-        assertThat(resourceAmount).isPresent();
-        assertThat(resourceAmount.get().getResource()).isEqualTo("A");
-        assertThat(resourceAmount.get().getAmount()).isEqualTo(7);
-    }
-
     @Test
     void shouldStillBeAbleToRetrieveByResourceWhenRemovingPartly() {
         // Arrange
@@ -340,43 +298,21 @@ void shouldNotBeAbleToRetrieveByResourceWhenRemovingCompletely() {
         assertThat(resourceAmount).isNotPresent();
     }
 
-    @Test
-    void shouldNotBeAbleToRetrieveByIdWhenRemovingCompletely() {
-        // Arrange
-        final ResourceListOperationResult<String> result = list.add("A", 10);
-        list.remove("A", 10);
-
-        // Act
-        final Optional<ResourceAmount<String>> resourceAmount = list.get(result.id());
-
-        // Assert
-        assertThat(resourceAmount).isNotPresent();
-    }
-
     @Test
     void shouldClearList() {
         // Arrange
-        final UUID id1 = list.add("A", 10).id();
-        final UUID id2 = list.add("B", 5).id();
+        list.add("A", 10);
+        list.add("B", 5);
 
         final Collection<ResourceAmount<String>> contentsBeforeClear = new ArrayList<>(list.getAll());
-        final Optional<ResourceAmount<String>> aBeforeClear = list.get(id1);
-        final Optional<ResourceAmount<String>> bBeforeClear = list.get(id2);
 
         // Act
         list.clear();
 
         // Assert
         final Collection<ResourceAmount<String>> contentsAfterClear = list.getAll();
-        final Optional<ResourceAmount<String>> aAfterClear = list.get(id1);
-        final Optional<ResourceAmount<String>> bAfterClear = list.get(id2);
 
         assertThat(contentsBeforeClear).hasSize(2);
-        assertThat(aBeforeClear).isPresent();
-        assertThat(bBeforeClear).isPresent();
-
         assertThat(contentsAfterClear).isEmpty();
-        assertThat(aAfterClear).isEmpty();
-        assertThat(bAfterClear).isEmpty();
     }
 }