Integrate feedback from code review (#300)

- Sort dependencies alphabetically - Rename classes and flux commands for consistency with others - Use backticks for options and their defaults in `@Description` - Remove unused code, add final modifiers See https://gitlab.com/oersi/oersi-etl/-/issues/238
metafacture · May 16, 2023 · 172a701 · 172a701
1 parent 81c1b26
commit 172a701
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 41 deletions.
diff --git a/metafix/build.gradle b/metafix/build.gradle
@@ -32,11 +32,11 @@ dependencies {
 
   implementation "org.metafacture:metafacture-commons:${versions.metafacture}"
   implementation "org.metafacture:metafacture-flowcontrol:${versions.metafacture}"
+  implementation "org.metafacture:metafacture-formatting:${versions.metafacture}"
   implementation "org.metafacture:metafacture-framework:${versions.metafacture}"
   implementation "org.metafacture:metafacture-io:${versions.metafacture}"
   implementation "org.metafacture:metafacture-javaintegration:${versions.metafacture}"
   implementation "org.metafacture:metafacture-mangling:${versions.metafacture}"
-  implementation "org.metafacture:metafacture-formatting:${versions.metafacture}"
   implementation "org.metafacture:metafacture-triples:${versions.metafacture}"
   implementation "org.metafacture:metamorph:${versions.metafacture}"
 

diff --git a/...metafacture/metafix/MetafixListPaths.java → ...org/metafacture/metafix/ListFixPaths.java b/...metafacture/metafix/MetafixListPaths.java → ...org/metafacture/metafix/ListFixPaths.java
@@ -24,29 +24,29 @@
 import org.metafacture.triples.AbstractTripleSort.Compare;
 
 /**
- * Provide a user-friendly way to list all paths available for processing in fix (see also {@link MetafixListValues}).
+ * Provide a user-friendly way to list all paths available for processing in fix (see also {@link ListFixValues}).
  *
  * @author Fabian Steeg
  */
 @Description("Lists all paths found in the input records. These paths can be used in a Fix to address fields. Options: " +
-        "count (output occurence frequency of each path, sorted by highest frequency first; default: true), " +
-        "template (for formatting the internal triple structure; default: ${o}\t|\t${s} if count is true, else ${s})" +
-        "index (output individual repeated subfields and array elements with index numbers instead of '*'; default: false)")
+        "`count` (output occurence frequency of each path, sorted by highest frequency first; default: `true`), " +
+        "`template` (for formatting the internal triple structure; default: `${o}\t|\t${s}` if count is true, else `${s}`)" +
+        "`index` (output individual repeated subfields and array elements with index numbers instead of '*'; default: `false`)")
 @In(StreamReceiver.class)
 @Out(String.class)
-@FluxCommand("fix-list-paths")
-public class MetafixListPaths extends MetafixStreamAnalyzer {
+@FluxCommand("list-fix-paths")
+public class ListFixPaths extends MetafixStreamAnalyzer {
 
-    public MetafixListPaths() {
+    public ListFixPaths() {
         super("nothing()", Compare.PREDICATE);
         setIndex(false);
     }
 
     public void setIndex(final boolean index) {
-        super.getFix().setEntityMemberName(index ? Metafix.DEFAULT_ENTITY_MEMBER_NAME : "*");
+        getFix().setEntityMemberName(index ? Metafix.DEFAULT_ENTITY_MEMBER_NAME : "*");
     }
 
     public boolean getIndex() {
-        return super.getFix().getEntityMemberName().equals(Metafix.DEFAULT_ENTITY_MEMBER_NAME);
+        return getFix().getEntityMemberName().equals(Metafix.DEFAULT_ENTITY_MEMBER_NAME);
     }
 }
diff --git a/...etafacture/metafix/MetafixListValues.java → ...rg/metafacture/metafix/ListFixValues.java b/...etafacture/metafix/MetafixListValues.java → ...rg/metafacture/metafix/ListFixValues.java
@@ -24,19 +24,19 @@
 import org.metafacture.triples.AbstractTripleSort.Compare;
 
 /**
- * Provide a user-friendly way to list all values for a given path (see {@link MetafixListPaths}).
+ * Provide a user-friendly way to list all values for a given path (see {@link ListFixPaths}).
  *
  * @author Fabian Steeg
  */
 @Description("Lists all values found for the given path. The paths can be found using fix-list-paths. Options: " +
-        "count (output occurence frequency of each value, sorted by highest frequency first; default: true)" +
-        "template (for formatting the internal triple structure; default: ${o}\t|\t${s} if count is true, else ${s})")
+        "`count` (output occurence frequency of each value, sorted by highest frequency first; default: `true`)" +
+        "`template` (for formatting the internal triple structure; default: `${o}\t|\t${s}` if count is true, else `${s}`)")
 @In(StreamReceiver.class)
 @Out(String.class)
-@FluxCommand("fix-list-values")
-public class MetafixListValues extends MetafixStreamAnalyzer {
+@FluxCommand("list-fix-values")
+public class ListFixValues extends MetafixStreamAnalyzer {
 
-    public MetafixListValues(final String path) {
+    public ListFixValues(final String path) {
         super(fix(path), Compare.OBJECT);
     }
 

diff --git a/metafix/src/main/java/org/metafacture/metafix/MetafixStreamAnalyzer.java b/metafix/src/main/java/org/metafacture/metafix/MetafixStreamAnalyzer.java
@@ -39,9 +39,9 @@
     private static final String DEFAULT_COUNTED_TEMPLATE = "${o}\t|\t${s}";
     private static final String DEFAULT_UNCOUNTED_TEMPLATE = "${s}";
 
-    private Metafix fix;
+    private final Metafix fix;
     private boolean count = true;
-    private Compare countBy;
+    private final Compare countBy;
     private String template;
 
     /* package-private */ MetafixStreamAnalyzer(final String fix, final Compare countBy) {
@@ -136,7 +136,4 @@ public String getTemplate() {
         return this.fix;
     }
 
-    /* package-private */ void setFix(final Metafix fix) {
-        this.fix = fix;
-    }
 }
diff --git a/metafix/src/main/resources/flux-commands.properties b/metafix/src/main/resources/flux-commands.properties
@@ -13,5 +13,5 @@
 # limitations under the License.
 #
 fix org.metafacture.metafix.Metafix
-fix-list-paths org.metafacture.metafix.MetafixListPaths
-fix-list-values org.metafacture.metafix.MetafixListValues
+list-fix-paths org.metafacture.metafix.ListFixPaths
+list-fix-values org.metafacture.metafix.ListFixValues
diff --git a/...facture/metafix/MetafixListPathsTest.java → ...metafacture/metafix/ListFixPathsTest.java b/...facture/metafix/MetafixListPathsTest.java → ...metafacture/metafix/ListFixPathsTest.java
@@ -26,21 +26,21 @@
 import org.mockito.exceptions.base.MockitoAssertionError;
 
 /**
- * Tests for class {@link MetafixListPaths}.
+ * Tests for class {@link ListFixPaths}.
  *
  * @author Fabian Steeg
  *
  */
-public final class MetafixListPathsTest {
+public final class ListFixPathsTest {
 
-    private MetafixListPaths lister;
+    private ListFixPaths lister;
 
     @Mock
     private ObjectReceiver<String> receiver;
 
-    public MetafixListPathsTest() {
+    public ListFixPathsTest() {
         MockitoAnnotations.initMocks(this);
-        lister = new MetafixListPaths();
+        lister = new ListFixPaths();
     }
 
     @Test

diff --git a/...acture/metafix/MetafixListValuesTest.java → ...etafacture/metafix/ListFixValuesTest.java b/...acture/metafix/MetafixListValuesTest.java → ...etafacture/metafix/ListFixValuesTest.java
@@ -26,66 +26,66 @@
 import org.mockito.exceptions.base.MockitoAssertionError;
 
 /**
- * Tests for class {@link MetafixListPaths}.
+ * Tests for class {@link ListFixValues}.
  *
  * @author Fabian Steeg
  *
  */
-public final class MetafixListValuesTest {
+public final class ListFixValuesTest {
 
-    private MetafixListValues lister;
+    private ListFixValues lister;
 
     @Mock
     private ObjectReceiver<String> receiver;
 
-    public MetafixListValuesTest() {
+    public ListFixValuesTest() {
         MockitoAnnotations.initMocks(this);
     }
 
     @Test
     public void testShouldListPathsSimple() {
-        lister = new MetafixListValues("a");
+        lister = new ListFixValues("a");
         verify("2\t|\taA");
     }
 
     @Test
     public void testShouldListPathsWildcard() {
-        lister = new MetafixListValues("a.*");
+        lister = new ListFixValues("a.*");
         verify("2\t|\taA");
     }
 
     @Test
     public void testShouldListPathsIndex() {
-        lister = new MetafixListValues("a.1");
+        lister = new ListFixValues("a.1");
         verify("1\t|\taA");
     }
 
     @Test
     public void testShouldListPathsTwoValues() {
-        lister = new MetafixListValues("b");
+        lister = new ListFixValues("b");
         verify(
                 "2\t|\tbB",
                 "1\t|\tbA");
     }
 
     @Test
     public void testShouldListPathsTwoValuesWildcard() {
-        lister = new MetafixListValues("b.*");
+        lister = new ListFixValues("b.*");
         verify(
             "2\t|\tbB",
             "1\t|\tbA");
     }
 
     @Test
     public void testShouldListPathsTwoValuesIndex() {
-        lister = new MetafixListValues("b.1");
+        lister = new ListFixValues("b.1");
         verify(
             "1\t|\tbB");
     }
 
     @Test
     public void testShouldListPathsSortCount() {
-        lister = new MetafixListValues("c");
+        lister = new ListFixValues("c");
         verify(
             "3\t|\tcC",
             "1\t|\tcA",
@@ -94,7 +94,7 @@ public void testShouldListPathsSortCount() {
 
     @Test
     public void testShouldListPathsDontCount() {
-        lister = new MetafixListValues("c");
+        lister = new ListFixValues("c");
         lister.setCount(false);
         verify(
             "cA",
@@ -120,7 +120,6 @@ private void processRecord() {
     }
 
     private void verify(final String... result) throws MockitoAssertionError {
-        lister.setTemplate(lister.getCount() ? "${o}\t|\t${s}" : "${s}");
         processRecord();
         try {
             final InOrder ordered = Mockito.inOrder(receiver);