Skip to content

Commit 172a701

Browse files
committed
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
1 parent 81c1b26 commit 172a701

File tree

7 files changed

+37
-41
lines changed

7 files changed

+37
-41
lines changed

metafix/build.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ dependencies {
3232

3333
implementation "org.metafacture:metafacture-commons:${versions.metafacture}"
3434
implementation "org.metafacture:metafacture-flowcontrol:${versions.metafacture}"
35+
implementation "org.metafacture:metafacture-formatting:${versions.metafacture}"
3536
implementation "org.metafacture:metafacture-framework:${versions.metafacture}"
3637
implementation "org.metafacture:metafacture-io:${versions.metafacture}"
3738
implementation "org.metafacture:metafacture-javaintegration:${versions.metafacture}"
3839
implementation "org.metafacture:metafacture-mangling:${versions.metafacture}"
39-
implementation "org.metafacture:metafacture-formatting:${versions.metafacture}"
4040
implementation "org.metafacture:metafacture-triples:${versions.metafacture}"
4141
implementation "org.metafacture:metamorph:${versions.metafacture}"
4242

metafix/src/main/java/org/metafacture/metafix/MetafixListPaths.java renamed to metafix/src/main/java/org/metafacture/metafix/ListFixPaths.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,29 +24,29 @@
2424
import org.metafacture.triples.AbstractTripleSort.Compare;
2525

2626
/**
27-
* Provide a user-friendly way to list all paths available for processing in fix (see also {@link MetafixListValues}).
27+
* Provide a user-friendly way to list all paths available for processing in fix (see also {@link ListFixValues}).
2828
*
2929
* @author Fabian Steeg
3030
*/
3131
@Description("Lists all paths found in the input records. These paths can be used in a Fix to address fields. Options: " +
32-
"count (output occurence frequency of each path, sorted by highest frequency first; default: true), " +
33-
"template (for formatting the internal triple structure; default: ${o}\t|\t${s} if count is true, else ${s})" +
34-
"index (output individual repeated subfields and array elements with index numbers instead of '*'; default: false)")
32+
"`count` (output occurence frequency of each path, sorted by highest frequency first; default: `true`), " +
33+
"`template` (for formatting the internal triple structure; default: `${o}\t|\t${s}` if count is true, else `${s}`)" +
34+
"`index` (output individual repeated subfields and array elements with index numbers instead of '*'; default: `false`)")
3535
@In(StreamReceiver.class)
3636
@Out(String.class)
37-
@FluxCommand("fix-list-paths")
38-
public class MetafixListPaths extends MetafixStreamAnalyzer {
37+
@FluxCommand("list-fix-paths")
38+
public class ListFixPaths extends MetafixStreamAnalyzer {
3939

40-
public MetafixListPaths() {
40+
public ListFixPaths() {
4141
super("nothing()", Compare.PREDICATE);
4242
setIndex(false);
4343
}
4444

4545
public void setIndex(final boolean index) {
46-
super.getFix().setEntityMemberName(index ? Metafix.DEFAULT_ENTITY_MEMBER_NAME : "*");
46+
getFix().setEntityMemberName(index ? Metafix.DEFAULT_ENTITY_MEMBER_NAME : "*");
4747
}
4848

4949
public boolean getIndex() {
50-
return super.getFix().getEntityMemberName().equals(Metafix.DEFAULT_ENTITY_MEMBER_NAME);
50+
return getFix().getEntityMemberName().equals(Metafix.DEFAULT_ENTITY_MEMBER_NAME);
5151
}
5252
}

metafix/src/main/java/org/metafacture/metafix/MetafixListValues.java renamed to metafix/src/main/java/org/metafacture/metafix/ListFixValues.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,19 @@
2424
import org.metafacture.triples.AbstractTripleSort.Compare;
2525

2626
/**
27-
* Provide a user-friendly way to list all values for a given path (see {@link MetafixListPaths}).
27+
* Provide a user-friendly way to list all values for a given path (see {@link ListFixPaths}).
2828
*
2929
* @author Fabian Steeg
3030
*/
3131
@Description("Lists all values found for the given path. The paths can be found using fix-list-paths. Options: " +
32-
"count (output occurence frequency of each value, sorted by highest frequency first; default: true)" +
33-
"template (for formatting the internal triple structure; default: ${o}\t|\t${s} if count is true, else ${s})")
32+
"`count` (output occurence frequency of each value, sorted by highest frequency first; default: `true`)" +
33+
"`template` (for formatting the internal triple structure; default: `${o}\t|\t${s}` if count is true, else `${s}`)")
3434
@In(StreamReceiver.class)
3535
@Out(String.class)
36-
@FluxCommand("fix-list-values")
37-
public class MetafixListValues extends MetafixStreamAnalyzer {
36+
@FluxCommand("list-fix-values")
37+
public class ListFixValues extends MetafixStreamAnalyzer {
3838

39-
public MetafixListValues(final String path) {
39+
public ListFixValues(final String path) {
4040
super(fix(path), Compare.OBJECT);
4141
}
4242

metafix/src/main/java/org/metafacture/metafix/MetafixStreamAnalyzer.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@
3939
private static final String DEFAULT_COUNTED_TEMPLATE = "${o}\t|\t${s}";
4040
private static final String DEFAULT_UNCOUNTED_TEMPLATE = "${s}";
4141

42-
private Metafix fix;
42+
private final Metafix fix;
4343
private boolean count = true;
44-
private Compare countBy;
44+
private final Compare countBy;
4545
private String template;
4646

4747
/* package-private */ MetafixStreamAnalyzer(final String fix, final Compare countBy) {
@@ -136,7 +136,4 @@ public String getTemplate() {
136136
return this.fix;
137137
}
138138

139-
/* package-private */ void setFix(final Metafix fix) {
140-
this.fix = fix;
141-
}
142139
}

metafix/src/main/resources/flux-commands.properties

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@
1313
# limitations under the License.
1414
#
1515
fix org.metafacture.metafix.Metafix
16-
fix-list-paths org.metafacture.metafix.MetafixListPaths
17-
fix-list-values org.metafacture.metafix.MetafixListValues
16+
list-fix-paths org.metafacture.metafix.ListFixPaths
17+
list-fix-values org.metafacture.metafix.ListFixValues

metafix/src/test/java/org/metafacture/metafix/MetafixListPathsTest.java renamed to metafix/src/test/java/org/metafacture/metafix/ListFixPathsTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,21 @@
2626
import org.mockito.exceptions.base.MockitoAssertionError;
2727

2828
/**
29-
* Tests for class {@link MetafixListPaths}.
29+
* Tests for class {@link ListFixPaths}.
3030
*
3131
* @author Fabian Steeg
3232
*
3333
*/
34-
public final class MetafixListPathsTest {
34+
public final class ListFixPathsTest {
3535

36-
private MetafixListPaths lister;
36+
private ListFixPaths lister;
3737

3838
@Mock
3939
private ObjectReceiver<String> receiver;
4040

41-
public MetafixListPathsTest() {
41+
public ListFixPathsTest() {
4242
MockitoAnnotations.initMocks(this);
43-
lister = new MetafixListPaths();
43+
lister = new ListFixPaths();
4444
}
4545

4646
@Test

metafix/src/test/java/org/metafacture/metafix/MetafixListValuesTest.java renamed to metafix/src/test/java/org/metafacture/metafix/ListFixValuesTest.java

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,66 +26,66 @@
2626
import org.mockito.exceptions.base.MockitoAssertionError;
2727

2828
/**
29-
* Tests for class {@link MetafixListPaths}.
29+
* Tests for class {@link ListFixValues}.
3030
*
3131
* @author Fabian Steeg
3232
*
3333
*/
34-
public final class MetafixListValuesTest {
34+
public final class ListFixValuesTest {
3535

36-
private MetafixListValues lister;
36+
private ListFixValues lister;
3737

3838
@Mock
3939
private ObjectReceiver<String> receiver;
4040

41-
public MetafixListValuesTest() {
41+
public ListFixValuesTest() {
4242
MockitoAnnotations.initMocks(this);
4343
}
4444

4545
@Test
4646
public void testShouldListPathsSimple() {
47-
lister = new MetafixListValues("a");
47+
lister = new ListFixValues("a");
4848
verify("2\t|\taA");
4949
}
5050

5151
@Test
5252
public void testShouldListPathsWildcard() {
53-
lister = new MetafixListValues("a.*");
53+
lister = new ListFixValues("a.*");
5454
verify("2\t|\taA");
5555
}
5656

5757
@Test
5858
public void testShouldListPathsIndex() {
59-
lister = new MetafixListValues("a.1");
59+
lister = new ListFixValues("a.1");
6060
verify("1\t|\taA");
6161
}
6262

6363
@Test
6464
public void testShouldListPathsTwoValues() {
65-
lister = new MetafixListValues("b");
65+
lister = new ListFixValues("b");
6666
verify(
6767
"2\t|\tbB",
6868
"1\t|\tbA");
6969
}
7070

7171
@Test
7272
public void testShouldListPathsTwoValuesWildcard() {
73-
lister = new MetafixListValues("b.*");
73+
lister = new ListFixValues("b.*");
7474
verify(
7575
"2\t|\tbB",
7676
"1\t|\tbA");
7777
}
7878

7979
@Test
8080
public void testShouldListPathsTwoValuesIndex() {
81-
lister = new MetafixListValues("b.1");
81+
lister = new ListFixValues("b.1");
8282
verify(
8383
"1\t|\tbB");
8484
}
8585

8686
@Test
8787
public void testShouldListPathsSortCount() {
88-
lister = new MetafixListValues("c");
88+
lister = new ListFixValues("c");
8989
verify(
9090
"3\t|\tcC",
9191
"1\t|\tcA",
@@ -94,7 +94,7 @@ public void testShouldListPathsSortCount() {
9494

9595
@Test
9696
public void testShouldListPathsDontCount() {
97-
lister = new MetafixListValues("c");
97+
lister = new ListFixValues("c");
9898
lister.setCount(false);
9999
verify(
100100
"cA",
@@ -120,7 +120,6 @@ private void processRecord() {
120120
}
121121

122122
private void verify(final String... result) throws MockitoAssertionError {
123-
lister.setTemplate(lister.getCount() ? "${o}\t|\t${s}" : "${s}");
124123
processRecord();
125124
try {
126125
final InOrder ordered = Mockito.inOrder(receiver);

0 commit comments

Comments
 (0)