Refactor/102_refactor_ColumnsDefinitionRenderer (#120)

* #102: Refactor ColumnsDefinitionRenderer to remove the leave() method * Add tests with empty column list * Update changelog * Implement review findings * Upgrade github-pages-deploy-action
exasol · Sep 27, 2021 · f4338fb · f4338fb
1 parent ab65afe
commit f4338fb
Show file tree

Hide file tree

Showing 7 changed files with 23 additions and 18 deletions.
diff --git a/.github/workflows/deploy_javadocs.yml b/.github/workflows/deploy_javadocs.yml
@@ -14,7 +14,7 @@ jobs:
       - name: Build Javadocs
         run: mvn javadoc:javadoc
       - name: Deploy
-        uses: JamesIves/github-pages-deploy-action@4.1.1
+        uses: JamesIves/github-pages-deploy-action@4.1.5
         with:
           branch: gh-pages
           folder: target/site/apidocs/
diff --git a/doc/changes/changes_4.4.3.md b/doc/changes/changes_4.4.3.md
@@ -4,7 +4,8 @@ Code name: Internal refactorings on "More Predicates"
 
 ## Refactoring
 
-* [#117](https://github.com/exasol/sql-statement-builder/issues/117): Fixed sonar findings
+* #117: Fixed sonar findings
+* #102: Refactored ColumnsDefinitionRenderer
 
 ## Dependency Updates
 

diff --git a/src/main/java/com/exasol/sql/ColumnDefinitionVisitor.java b/src/main/java/com/exasol/sql/ColumnDefinitionVisitor.java
@@ -6,8 +6,6 @@
 public interface ColumnDefinitionVisitor {
     public void visit(final ColumnsDefinition columnsDefinition);
 
-    public void leave(final ColumnsDefinition columnsDefinition);
-
     public void visit(final Column column);
 
     public void visit(final Char charColumn);

diff --git a/src/main/java/com/exasol/sql/ColumnsDefinition.java b/src/main/java/com/exasol/sql/ColumnsDefinition.java
@@ -23,7 +23,7 @@ public ColumnsDefinition(final SqlStatement root) {
 
     /**
      * Create an new instance of {@link ColumnsDefinition}.
-     * 
+     *
      * @param columns list of columns
      */
     public ColumnsDefinition(final List<Column> columns) {
@@ -33,7 +33,7 @@ public ColumnsDefinition(final List<Column> columns) {
 
     /**
      * Create a new builder for {@link ColumnsDefinition}.
-     * 
+     *
      * @return new {@link Builder}
      */
     public static Builder builder() {
@@ -52,7 +52,7 @@ public void add(final String name, final DataType dataType) {
 
     /**
      * Get a list of columns.
-     * 
+     *
      * @return list of columns
      */
     public List<Column> getColumns() {
@@ -61,10 +61,6 @@ public List<Column> getColumns() {
 
     public void accept(final ColumnDefinitionVisitor visitor) {
         visitor.visit(this);
-        for (final Column column : this.columns) {
-            column.accept(visitor);
-        }
-        visitor.leave(this);
     }
 
     /**

diff --git a/src/main/java/com/exasol/sql/rendering/ColumnsDefinitionRenderer.java b/src/main/java/com/exasol/sql/rendering/ColumnsDefinitionRenderer.java
@@ -1,5 +1,7 @@
 package com.exasol.sql.rendering;
 
+import java.util.List;
+
 import com.exasol.datatype.type.*;
 import com.exasol.datatype.type.Boolean;
 import com.exasol.sql.*;
@@ -19,14 +21,10 @@ public ColumnsDefinitionRenderer(final StringRendererConfig config) {
 
     @Override
     public void visit(final ColumnsDefinition columnsDefinition) {
-        if (!columnsDefinition.getColumns().isEmpty()) {
+        final List<Column> columns = columnsDefinition.getColumns();
+        if (!columns.isEmpty()) {
             append("(");
-        }
-    }
-
-    @Override
-    public void leave(final ColumnsDefinition columnsDefinition) {
-        if (!columnsDefinition.getColumns().isEmpty()) {
+            columns.forEach(col -> col.accept(this));
             append(")");
         }
         setLastVisited(columnsDefinition);

diff --git a/src/test/java/com/exasol/sql/ddl/create/rendering/TestCreateTableRendering.java b/src/test/java/com/exasol/sql/ddl/create/rendering/TestCreateTableRendering.java
@@ -72,6 +72,12 @@ void testCreateTableWithInvalidCharSizeColumn() {
         assertThrows(IllegalArgumentException.class, () -> this.createTable.charColumn("a", -1));
     }
 
+    @Test
+    // Not a requirement, just to see what happens
+    void testCreateTableWithoutColumn() {
+        assertThat(this.createTable, rendersTo("CREATE TABLE testName"));
+    }
+
     @Test
     void testCreateTableWithVarcharColumn() {
         assertThat(this.createTable.varcharColumn("a", 3000), rendersTo("CREATE TABLE testName (a VARCHAR(3000))"));

diff --git a/src/test/java/com/exasol/sql/dql/select/rendering/TestSelectRendering.java b/src/test/java/com/exasol/sql/dql/select/rendering/TestSelectRendering.java
@@ -28,6 +28,12 @@ void beforeEach() {
         this.select = StatementFactory.getInstance().select();
     }
 
+    @Test
+    // Not a requirement, just to see what happens
+    void testSelectWithoutFields() {
+        assertThat(this.select, rendersTo("SELECT "));
+    }
+
     // [utest->dsn~rendering.sql.select~1]
     @Test
     void testSelectAll() {