Repository: decorators-squad/eo-yaml
Pull request #222 (2020-03-24 17:48:10)

Refactored AllYamlLines.toYamlNode(...)
PR for #218 

* removed repetitive cases
* replaces ``switch`` with ``if()``, it is clearer
* renamed class ``Nested`` to ``Follows`` and added javadocs
* It can also turn into a plain Scalar now
* it will throw ISE if the node cannot be figured out
* added unit tests


Patch:
diff --git a/src/main/java/com/amihaiemil/eoyaml/AllYamlLines.java b/src/main/java/com/amihaiemil/eoyaml/AllYamlLines.java
index d1e33ab..c93d23a 100644
--- a/src/main/java/com/amihaiemil/eoyaml/AllYamlLines.java
+++ b/src/main/java/com/amihaiemil/eoyaml/AllYamlLines.java
@@ -42,9 +42,6 @@ import java.util.List;
  * @author Mihai Andronache (amihaiemil@gmail.com)
  * @version $Id$
  * @since 1.0.0
- * @todo #211:30min Method toYamlNode has a lot of duplicated case, it
- *  most likely needs to be refactored. Make sure to add tests for all
- *  the performed changes as this method is quite important in the library.
  */
 final class AllYamlLines implements YamlLines {
 
@@ -102,56 +99,44 @@ final class AllYamlLines implements YamlLines {
 
     @Override
     public YamlNode toYamlNode(final YamlLine prev) {
-        final String trimmed = prev.trimmed();
-        final String last = trimmed.substring(trimmed.length()-1);
         final YamlNode node;
-        switch (last) {
-            case Nested.YAML:
-                final boolean sequence = this.iterator()
-                    .next().trimmed().startsWith("-");
-                if(sequence) {
-                    node = new ReadYamlSequence(this);
-                } else {
-                    node = new ReadYamlMapping(this);
-                }
-                break;
-            case Nested.KEY_YAML:
-                final boolean sequenceKey = this.iterator()
-                    .next().trimmed().startsWith("-");
-                if(sequenceKey) {
-                    node = new ReadYamlSequence(this);
-                } else {
-                    node = new ReadYamlMapping(this);
-                }
-                break;
-            case Nested.SEQUENCE:
-                if(trimmed.length() == 1 || "---".equals(trimmed)) {
-                    final boolean elementSequence = this.iterator()
-                        .next().trimmed().startsWith("-");
-                    if(elementSequence) {
-                        node = new ReadYamlSequence(this);
-                    } else {
-                        node = new ReadYamlMapping(this);
-                    }
-                } else {
-                    node = new ReadYamlSequence(this);
-                }
-                break;
-            case Nested.LITERAL_BLOCK_SCALAR:
-                node = new ReadLiteralBlockScalar(this);
-                break;
-            case Nested.FOLDED_BLOCK_SCALAR:
-                node = new ReadFoldedBlockScalar(this);
-                break;
-            default:
-                final boolean elementSequence = this.iterator()
-                    .next().trimmed().startsWith("-");
-                if(elementSequence) {
-                    node = new ReadYamlSequence(this);
-                } else {
-                    node = new ReadYamlMapping(this);
-                }
-                break;
+        final String prevLine = prev.trimmed();
+        final String lastChar = prevLine.substring(prevLine.length()-1);
+
+        if(lastChar.equals(Follows.LITERAL_BLOCK_SCALAR)) {
+            node = new ReadLiteralBlockScalar(this);
+        } else if (lastChar.equals(Follows.FOLDED_BLOCK_SCALAR)) {
+            node = new ReadFoldedBlockScalar(this);
+        } else if(prevLine.matches(Follows.FOLDED_SEQUENCE)) {
+            node = new ReadYamlSequence(this);
+        } else {
+            node = this.mappingSequenceOrPlainScalar();
+        }
+        return node;
+    }
+
+    /**
+     * Try to figure out what YAML node (mapping, sequence or scalar) do these
+     * lines represent.
+     * @return Found YamlNode.
+     */
+    private YamlNode mappingSequenceOrPlainScalar() {
+        final YamlNode node;
+        final YamlLine first = this.iterator().next();
+        if(first.trimmed().startsWith("-")) {
+            node = new ReadYamlSequence(this);
+        } else if (first.trimmed().contains(":")){
+            node = new ReadYamlMapping(this);
+        } else if(this.lines().size() == 1) {
+            node = new ReadPlainScalarValue(first);
+        } else {
+            throw new IllegalStateException(
+                "Could not parse YAML starting at line " + (first.number() + 1)
+              + " . It should be a sequence (line should start with '-'), "
+              + "a mapping (line should contain ':') or it should be a plain "
+              + "scalar, but it has " + this.lines.size() + " lines, "
+              + "while a plain scalar should be only 1 line!"
+            );
         }
         return node;
     }
diff --git a/src/main/java/com/amihaiemil/eoyaml/Nested.java b/src/main/java/com/amihaiemil/eoyaml/Follows.java
similarity index 67%
rename from src/main/java/com/amihaiemil/eoyaml/Nested.java
rename to src/main/java/com/amihaiemil/eoyaml/Follows.java
index a5f9b49..af6f85c 100644
--- a/src/main/java/com/amihaiemil/eoyaml/Nested.java
+++ b/src/main/java/com/amihaiemil/eoyaml/Follows.java
@@ -28,49 +28,56 @@
 package com.amihaiemil.eoyaml;
 
 /**
- * Nested Yaml types.
+ * Nested Yaml types. If a YAML line ends with one
+ * of these, it means a certain type of YAML Node follows.
  * @author Mihai Andronache (amihaiemil@gmail.com)
  * @version $Id$
  * @since 1.0.2
  */
-final class Nested {
+final class Follows {
 
     /**
      * Hidden ctor.
      */
-    private Nested() {}
+    private Follows() {
+    }
 
     /**
-     * If this is the last char on a line, it means a Yaml
-     * node should be nested bellow.
-     */
-    static final String YAML = ":";
-
-    /**
-     * If this is the last char on a line, it means that
-     * 1) a Yaml sequence should be wrapped bellow (looks the same as a normal
-     *    one, but the char '-' is ommitted from the beginning of its lines).
-     * or
-     * 2) a Yaml node is bellow it (an element from the current sequence).
-     */
-    static final String SEQUENCE = "-";
-
-    /**
-     * If this is the last char on a line, it means a pointed wrapped scalar
+     * If this is the last char on a line, it means a folded block scalar
      * should be nested bellow (see Example 2.15 from YAML spec 1.2).
+     * <pre>
+     *   foldedScalar: >
+     *     a long line split into
+     *     several short
+     *     lines for readability
+     * </pre>
      */
     static final String FOLDED_BLOCK_SCALAR = ">";
 
     /**
      * If this is the last char on a line, it means a literal block scalar
-     * should be nested bellow (all newlines are significant, and
-     * taken into account).
+     * should be nested bellow.
+     * <pre>
+     *   literalScalar: |
+     *     line 1
+     *     line 2
+     *     line 3
+     * </pre>
      */
     static final String LITERAL_BLOCK_SCALAR = "|";
 
     /**
-     * If this is the last char on a line, it means a complex key
-     * (mapping or sequence) should be nested bellow.
+     * If the line ends with this, it means a folded sequence follows after it.
+     * This is a RegEx pattern because we want it to work even if there are
+     * spaces between the | and the -. Both "| -" and "|-" line endings
+     * should be fine.
+     * E.g.
+     * <pre>
+     *     foldedSequence: |-
+     *       some
+     *       sequence
+     *       values
+     * </pre>
      */
-    static final String KEY_YAML = "?";
+    static final String FOLDED_SEQUENCE = "^.+\\|[ ]*\\-$";
 }
diff --git a/src/main/java/com/amihaiemil/eoyaml/ReadPlainScalarValue.java b/src/main/java/com/amihaiemil/eoyaml/ReadPlainScalarValue.java
index 67b14be..a76d6ac 100644
--- a/src/main/java/com/amihaiemil/eoyaml/ReadPlainScalarValue.java
+++ b/src/main/java/com/amihaiemil/eoyaml/ReadPlainScalarValue.java
@@ -60,12 +60,7 @@ final class ReadPlainScalarValue extends ComparableScalar {
         } else if(trimmed.contains(":") && !trimmed.endsWith(":")) {
             value = trimmed.substring(trimmed.indexOf(":") + 1).trim();
         } else {
-            throw new IllegalStateException(
-                "Missing scalar on line " + (this.line.number() + 1) + ". "
-                + "The line was expected to be either part of a "
-                + "mapping (key: value) or part of a sequence (- value). "
-                + "Instead, the line is: [" + this.line.trimmed() + "]."
-            );
+            value = trimmed;
         }
         return this.unescape(value);
     }


Test patch:
diff --git a/src/test/java/com/amihaiemil/eoyaml/AllYamlLinesTest.java b/src/test/java/com/amihaiemil/eoyaml/AllYamlLinesTest.java
index bbb7080..51111cf 100644
--- a/src/test/java/com/amihaiemil/eoyaml/AllYamlLinesTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/AllYamlLinesTest.java
@@ -28,11 +28,12 @@
 package com.amihaiemil.eoyaml;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
-
 import org.hamcrest.MatcherAssert;
 import org.hamcrest.Matchers;
+import org.junit.Assert;
 import org.junit.Test;
 
 /**
@@ -95,4 +96,188 @@ public final class AllYamlLinesTest {
         MatcherAssert.assertThat(iterator.hasNext(), Matchers.is(false));
     }
 
+    /**
+     * Unit test for AllYamlLines.toYamlNode(). The lines should turn into a
+     * literal block scalar.
+     */
+    @Test
+    public void turnsIntoLiteralBlockScalar() {
+        final List<YamlLine> lines = new ArrayList<>();
+        lines.add(new RtYamlLine("line1", 1));
+        lines.add(new RtYamlLine("line2", 2));
+        lines.add(new RtYamlLine("line3", 3));
+        final YamlLines yamlLines = new AllYamlLines(lines);
+        MatcherAssert.assertThat(
+            yamlLines.toYamlNode(new RtYamlLine("literalScalar:|", 0)),
+            Matchers.instanceOf(ReadLiteralBlockScalar.class)
+        );
+    }
+
+    /**
+     * Unit test for AllYamlLines.toYamlNode(). The lines should turn into a
+     * folded block scalar.
+     */
+    @Test
+    public void turnsIntoFoldedBlockScalar() {
+        final List<YamlLine> lines = new ArrayList<>();
+        lines.add(new RtYamlLine("more lines for", 1));
+        lines.add(new RtYamlLine("better understanding", 2));
+        lines.add(new RtYamlLine("of text", 3));
+        final YamlLines yamlLines = new AllYamlLines(lines);
+        MatcherAssert.assertThat(
+            yamlLines.toYamlNode(new RtYamlLine("foldedScalar:>", 0)),
+            Matchers.instanceOf(ReadFoldedBlockScalar.class)
+        );
+    }
+
+    /**
+     * Unit test for AllYamlLines.toYamlNode(). The lines should turn into a
+     * (folded) sequence.
+     */
+    @Test
+    public void turnsIntoFoldedSequence() {
+        final List<YamlLine> lines = new ArrayList<>();
+        lines.add(new RtYamlLine("value1", 1));
+        lines.add(new RtYamlLine("value2", 2));
+        lines.add(new RtYamlLine("value3", 3));
+        final YamlLines yamlLines = new AllYamlLines(lines);
+        final YamlNode seq =  yamlLines.toYamlNode(
+            new RtYamlLine("foldedSequence:|-", 0)
+        );
+        MatcherAssert.assertThat(
+            seq, Matchers.instanceOf(ReadYamlSequence.class)
+        );
+        final Collection<YamlNode> values = ((YamlSequence) seq).values();
+        MatcherAssert.assertThat(values.size(), Matchers.is(3));
+        System.out.print(values);
+        MatcherAssert.assertThat(
+            values.contains(new BuiltPlainScalar("value1")),
+            Matchers.is(Boolean.TRUE)
+        );
+        MatcherAssert.assertThat(
+            values.contains(
+                new BuiltPlainScalar("value2")
+            ),
+            Matchers.is(Boolean.TRUE)
+        );
+        MatcherAssert.assertThat(
+            values.contains(new BuiltPlainScalar("value3")),
+            Matchers.is(Boolean.TRUE)
+        );
+    }
+
+    /**
+     * Unit test for AllYamlLines.toYamlNode(). The lines should turn into a
+     * (folded) sequence. There are spaces between the last 2 characters of
+     * the previous line.
+     */
+    @Test
+    public void turnsIntoFoldedSequenceWithSpaces() {
+        final List<YamlLine> lines = new ArrayList<>();
+        lines.add(new RtYamlLine("value1", 1));
+        lines.add(new RtYamlLine("value2", 2));
+        lines.add(new RtYamlLine("value3", 3));
+        final YamlLines yamlLines = new AllYamlLines(lines);
+        final YamlNode seq =  yamlLines.toYamlNode(
+            new RtYamlLine("foldedSequence:| -", 0)
+        );
+        MatcherAssert.assertThat(
+            seq, Matchers.instanceOf(ReadYamlSequence.class)
+        );
+        final Collection<YamlNode> values = ((YamlSequence) seq).values();
+        MatcherAssert.assertThat(values.size(), Matchers.is(3));
+        System.out.print(values);
+        MatcherAssert.assertThat(
+            values.contains(new BuiltPlainScalar("value1")),
+            Matchers.is(Boolean.TRUE)
+        );
+        MatcherAssert.assertThat(
+            values.contains(
+                new BuiltPlainScalar("value2")
+            ),
+            Matchers.is(Boolean.TRUE)
+        );
+        MatcherAssert.assertThat(
+            values.contains(new BuiltPlainScalar("value3")),
+            Matchers.is(Boolean.TRUE)
+        );
+    }
+
+    /**
+     * Unit test for AllYamlLines.toYamlNode(). The lines should turn
+     * into a (detected) mapping.
+     */
+    @Test
+    public void turnsIntoMapping() {
+        final List<YamlLine> lines = new ArrayList<>();
+        lines.add(new RtYamlLine("some: mapping", 1));
+        lines.add(new RtYamlLine("for: test", 2));
+        final YamlLines yamlLines = new AllYamlLines(lines);
+        MatcherAssert.assertThat(
+            yamlLines.toYamlNode(new RtYamlLine("---", 0)),
+            Matchers.instanceOf(ReadYamlMapping.class)
+        );
+    }
+
+    /**
+     * Unit test for AllYamlLines.toYamlNode(). The lines should turn
+     * into a (detected) sequence.
+     */
+    @Test
+    public void turnsIntoSequence() {
+        final List<YamlLine> lines = new ArrayList<>();
+        lines.add(new RtYamlLine("- some", 1));
+        lines.add(new RtYamlLine("- sequence", 2));
+        final YamlLines yamlLines = new AllYamlLines(lines);
+        MatcherAssert.assertThat(
+            yamlLines.toYamlNode(new RtYamlLine("?", 0)),
+            Matchers.instanceOf(ReadYamlSequence.class)
+        );
+    }
+
+    /**
+     * Unit test for AllYamlLines.toYamlNode(). The lines should turn
+     * into a (detected) sequence.
+     */
+    @Test
+    public void turnsIntoPlainScalar() {
+        final List<YamlLine> lines = new ArrayList<>();
+        lines.add(new RtYamlLine("justAScalar", 1));
+        final YamlLines yamlLines = new AllYamlLines(lines);
+        MatcherAssert.assertThat(
+            yamlLines.toYamlNode(new RtYamlLine("---", 0)),
+            Matchers.instanceOf(ReadPlainScalarValue.class)
+        );
+    }
+
+    /**
+     * Unit test for AllYamlLines.toYamlNode(). It should complain, because
+     * it cannot detect the type of node. The previous line does not point to
+     * any specific node and the lines themselves are not a mapping, sequence
+     * or a plains scalar.
+     */
+    @Test
+    public void complainsWhenCannotTurn() {
+        final List<YamlLine> lines = new ArrayList<>();
+        lines.add(new RtYamlLine("this will fail", 0));
+        lines.add(new RtYamlLine("because these lines", 1));
+        lines.add(new RtYamlLine("have no YAML meaning.", 2));
+        lines.add(new RtYamlLine("They are also not", 3));
+        lines.add(new RtYamlLine("a folded or literal scalar", 4));
+        final YamlLines yamlLines = new AllYamlLines(lines);
+        try {
+            yamlLines.toYamlNode(new RtYamlLine("---", -1));
+            Assert.fail("Expected IllegalStateException!");
+        } catch (final IllegalStateException ex) {
+            final String message = ex.getMessage();
+            MatcherAssert.assertThat(
+                message,
+                Matchers.startsWith("Could not parse YAML starting at line 1")
+            );
+            MatcherAssert.assertThat(
+                message,
+                Matchers.containsString("but it has 5 lines")
+            );
+        }
+    }
 }
diff --git a/src/test/java/com/amihaiemil/eoyaml/ReadPlainScalarValueTest.java b/src/test/java/com/amihaiemil/eoyaml/ReadPlainScalarValueTest.java
index afc8bf6..5cfa254 100644
--- a/src/test/java/com/amihaiemil/eoyaml/ReadPlainScalarValueTest.java
+++ b/src/test/java/com/amihaiemil/eoyaml/ReadPlainScalarValueTest.java
@@ -29,7 +29,6 @@ package com.amihaiemil.eoyaml;
 
 import org.hamcrest.MatcherAssert;
 import org.hamcrest.Matchers;
-import org.junit.Assert;
 import org.junit.Test;
 
 /**
@@ -63,37 +62,24 @@ public final class ReadPlainScalarValueTest {
     }
 
     /**
-     * ReadPlainScalarValue should throw an exception if the given line doesn't
-     * contain a scalar.
+     * ReadPlainScalarValue will return the plain scalar.
      */
-    @Test(expected = IllegalStateException.class)
-    public void missingScalarInSequence() {
-        final YamlLine line = new RtYamlLine("-", 0);
-        final Scalar scalar = new ReadPlainScalarValue(line);
-        scalar.value();
-        Assert.fail("IllegalStateException was expected.");
-    }
-
-    /**
-     * ReadPlainScalarValue should throw an exception if the given line doesn't
-     * contain a scalar.
-     */
-    @Test(expected = IllegalStateException.class)
-    public void missingScalarInMapping() {
-        final YamlLine line = new RtYamlLine("key: ", 0);
+    @Test
+    public void returnsPlainScalar() {
+        final YamlLine line = new RtYamlLine("value", 0);
         final Scalar scalar = new ReadPlainScalarValue(line);
-        scalar.value();
-        Assert.fail("IllegalStateException was expected.");
+        MatcherAssert.assertThat(scalar.value(), Matchers.equalTo("value"));
     }
 
     /**
-     * ReadPlainScalarValue will complain if the given YamlLine is empty.
+     * ReadPlainScalarValue will return empty string if the line is empty.
+     * Should never be the case in real practice, however, because empty
+     * lines are omitted when read.
      */
-    @Test(expected = IllegalStateException.class)
-    public void complainsOnEmptyLine() {
+    @Test
+    public void returnsEmptyScalar() {
         final YamlLine line = new RtYamlLine("", 0);
         final Scalar scalar = new ReadPlainScalarValue(line);
-        scalar.value();
-        Assert.fail("IllegalStateException was expected.");
+        MatcherAssert.assertThat(scalar.value(), Matchers.isEmptyString());
     }
 }