(#1224) Making list/Immutable truly immutable

yegor256 · Nov 9, 2019 · 2eaf3c7 · 2eaf3c7
1 parent 2136d9d
commit 2eaf3c7
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 23 deletions.
diff --git a/src/main/java/org/cactoos/list/Immutable.java b/src/main/java/org/cactoos/list/Immutable.java
@@ -27,6 +27,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.ListIterator;
+import org.cactoos.collection.Sliced;
 
 /**
  * {@link List} envelope that doesn't allow mutations.
@@ -55,10 +56,18 @@ public final class Immutable<T> implements List<T> {
 
     /**
      * Ctor.
-     * @param src Source
+     * @param src Source collection
+     */
+    public Immutable(final Collection<T> src) {
+        this(new ListOf<>(src));
+    }
+
+    /**
+     * Ctor.
+     * @param src Source list
      */
     public Immutable(final List<T> src) {
-        this.list = src;
+        this.list = new ListOf<>(src);
     }
 
     @Override
@@ -198,7 +207,9 @@ public ListIterator<T> listIterator(final int index) {
 
     @Override
     public List<T> subList(final int start, final int end) {
-        return this.list.subList(start, end);
+        return new Immutable<>(
+            new Sliced<>(start, end - start, this.list)
+        );
     }
 
     @Override

diff --git a/src/test/java/org/cactoos/list/ImmutableTest.java b/src/test/java/org/cactoos/list/ImmutableTest.java
@@ -23,6 +23,9 @@
  */
 package org.cactoos.list;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 import org.cactoos.collection.CollectionOf;
 import org.hamcrest.core.IsEqual;
 import org.junit.Test;
@@ -38,9 +41,24 @@
  * @checkstyle MagicNumberCheck (500 lines)
  * @checkstyle ClassDataAbstractionCouplingCheck (500 lines)
  */
-@SuppressWarnings("PMD.TooManyMethods")
+@SuppressWarnings({"PMD.TooManyMethods", "PMD.AvoidDuplicateLiterals"})
 public class ImmutableTest {
 
+    @Test
+    public void testImmutability() {
+        final List<String> strings = new ArrayList<>(Arrays.asList("a", "b", "c"));
+        final List<String> immutable = new Immutable<>(strings);
+        final int original = immutable.size();
+        strings.add("d");
+        new Assertion<>(
+            "inner list must not be changed",
+            original,
+            new IsEqual<>(
+                immutable.size()
+            )
+        ).affirm();
+    }
+
     @Test
     public void size() {
         new Assertion<>(
@@ -372,6 +390,22 @@ public void subList() {
         ).affirm();
     }
 
+    @Test
+    public void immutableSubList() {
+        new Assertion<>(
+            "subList() must be immutable",
+            () -> new Immutable<>(
+                new ListOf<>("a", "b", "c")
+            ).subList(0, 2).add("d"),
+            new Throws<>(
+                new MatcherOf<>(
+                    (String msg) -> msg.equals("#add(T): the list is read-only")
+                ),
+                UnsupportedOperationException.class
+            )
+        ).affirm();
+    }
+
     @Test
     public void testToString() {
         new Assertion<>(

diff --git a/src/test/java/org/cactoos/list/ListEnvelopeTest.java b/src/test/java/org/cactoos/list/ListEnvelopeTest.java
@@ -26,13 +26,14 @@
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.LinkedList;
-import java.util.List;
 import java.util.ListIterator;
 import org.hamcrest.MatcherAssert;
 import org.hamcrest.Matchers;
 import org.hamcrest.core.IsEqual;
 import org.junit.Test;
 import org.llorllale.cactoos.matchers.Assertion;
+import org.llorllale.cactoos.matchers.MatcherOf;
+import org.llorllale.cactoos.matchers.Throws;
 
 /**
  * Test case for {@link ListEnvelope}.
@@ -41,6 +42,7 @@
  * @checkstyle JavadocMethodCheck (500 lines)
  * @checkstyle JavadocTypeCheck (500 lines)
  * @checkstyle MagicNumberCheck (500 lines)
+ * @checkstyle ClassDataAbstractionCouplingCheck (500 lines)
  * @todo #898:30min Get rid of the Immutable in StringList nested class
  *  That's because this test should check the original behavior of ListEnvelope
  *  Now this test checks behavior of the Immutable decorator
@@ -91,19 +93,25 @@ public void subListReturnsListIteratorWithUnsupportedRemove() {
 
     @Test()
     public void subListReturnsListIteratorWithSupportedSet() {
-        final ListIterator<String> iterator = new StringList("one", "two", "three")
-            .subList(0, 2)
-            .listIterator(0);
-        iterator.next();
-        iterator.set("zero");
-        iterator.previous();
-        MatcherAssert.assertThat(
-            "iterator is not equal to expected",
-            () -> iterator,
-            Matchers.contains(
-                "zero", "two"
+        new Assertion<>(
+            "subList.listIterator().set() must throw exception",
+            () -> {
+                final ListIterator<String> iterator = new StringList("one", "two", "three")
+                    .subList(0, 2)
+                    .listIterator(0);
+                iterator.next();
+                iterator.set("zero");
+                return new Object();
+            },
+            new Throws<>(
+                new MatcherOf<>(
+                    (String msg) -> msg.equals(
+                        "List Iterator is read-only and doesn't allow rewriting items"
+                    )
+                ),
+                UnsupportedOperationException.class
             )
-        );
+        ).affirm();
     }
 
     @Test(expected = UnsupportedOperationException.class)
@@ -141,13 +149,14 @@ public void returnsSubListWithUnsupportedRemove() {
 
     @Test()
     public void returnsSubListWithSupportedSet() {
-        final List<String> sublist = new StringList("one").subList(0, 1);
-        sublist.set(0, "zero");
         new Assertion<>(
-            "subList must be equal to expected",
-            sublist,
-            new IsEqual<>(
-                new ListOf<>("zero")
+            "subList.set() must throw exception",
+            () -> new StringList("one").subList(0, 1).set(0, "zero"),
+            new Throws<>(
+                new MatcherOf<>(
+                    (String msg) -> msg.equals("#set(): the list is read-only")
+                ),
+                UnsupportedOperationException.class
             )
         ).affirm();
     }