4
\$\begingroup\$

An endpoint that my application interacts with allows you to specify which fields are returned in the JSON result. My application will then map the JSON to a POJO using Jackson. Quite often I add more fields in one place and forget to add it in the other which results in problems. I wrote a unit test to help make the process easier but I feel like there could be room for improvements.

Here's what I have so far:

private static final String[] POTENTIAL_FIELDS = new String[] { "field1", "field2" };

@Test
public void checkFields() {
    // get all constant fields
    Set<String> constantFields = Arrays.stream(POTENTIAL_FIELDS)
            .map(String::toLowerCase)
            .collect(Collectors.toSet());

    // get all private fields
    Set<String> reflectFields = Arrays.stream(FieldsModel.class.getDeclaredFields())
            .filter((field) -> !Modifier.isStatic(field.getModifiers()))
            .map(field -> field.getName().toLowerCase())
            .collect(Collectors.toSet());

    Set<String> symmetricDifference = Sets.symmetricDifference(constantFields, reflectFields);

    // check if there's a difference
    if (!symmetricDifference.isEmpty()) {
        Assert.fail(String.format("Difference: %s\n", String.join(",", symmetricDifference)));
    }
}

private static class FieldsModel {
    private int field1;
    private String field2;
}

The only non-standard imports are FieldsModel which is what the name suggests, and Sets which is from Guava (com.google.coom.collect.Sets). Here is a direct link to the method I used.

\$\endgroup\$
  • \$\begingroup\$ "Here's what I have so far" Does it work like you want it? \$\endgroup\$ – Mast Jul 29 '19 at 16:36
  • \$\begingroup\$ Yes it does, it has the desired effect. I just was wondering if there was a more efficient way to find the disjunctive union than Guava's symmetricDifference or if I did something weird with Assert.fail(...) \$\endgroup\$ – Michael Ziluck Jul 29 '19 at 16:37
  • \$\begingroup\$ Do you know how Sets.symmetricDifference works? Is the algorithm available in literature or how do you suggest we can compare it to other methods? \$\endgroup\$ – dfhwze Jul 29 '19 at 18:46
  • 1
    \$\begingroup\$ @dfhwze Just edited my post to provide a direct link to that method. \$\endgroup\$ – Michael Ziluck Jul 29 '19 at 18:55
  • 1
    \$\begingroup\$ @dfhwze Edited again to include those \$\endgroup\$ – Michael Ziluck Jul 29 '19 at 19:33
3
\$\begingroup\$

On the whole, I think this is a pretty neat solution to your problem. A couple of observations...

case == CASE

You lower case everything. Maybe this is exactly what you're after, but to me, fieldOne and fieldone aren't the same, so I'd want to know about that difference.

Developer time vs test time

symmetricDifference gives back a set of all of the differences, it doesn't discriminate between the left and the right side. So, if you've added something to FieldsModel and something else to POTENTIAL_FIELDS they would both come back. The developer then has to go and look at both and reconcile where the problem is. In reality this might not be a problem because the developer has probably worked on one of the files recently, so would realise which one they'd missed, however it would be better if the test showed what was missing from where. You could do something like this:

String inConstantButNotModel = constantFields.stream()
        .filter(i -> !reflectFields.contains(i))
        .collect(Collectors.joining(","));
String inModelButNotConstant = reflectFields.stream()
        .filter(i -> !constantFields.contains(i))
        .collect(Collectors.joining(","));

assertThat(String.format("ConstantsMissing: %s\nModelMissing: %s\n",
        inModelButNotConstant,
        inConstantButNotModel))
        .isEqualTo("ConstantsMissing: \nModelMissing: \n");

To give output along the lines of...

ConstantsMissing: field5,field4
ModelMissing: field3
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ That is a really good point and quite clever. Thank you! I'm actually going to go back to open a PR for this project even though I've been off it for awhile. \$\endgroup\$ – Michael Ziluck Feb 20 at 14:57
0
\$\begingroup\$
  1. If you are using only one class there is no need to use ArrayList for POTENTIAL_FIELDS and then after converting it to set. Class will have a unique field name.
  2. If the number of class fields are not large then no need to worried about performance.
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ In the comments on my original question, they asked me to add in POTENTIAL_FIELDS so testers didn't need to. In the actual code, it's in a separate class. Also, where do you see an ArrayList? \$\endgroup\$ – Michael Ziluck Aug 2 '19 at 12:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.