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\$
6
  • \$\begingroup\$ "Here's what I have so far" Does it work like you want it? \$\endgroup\$
    – Mast
    Commented Jul 29, 2019 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\$ Commented Jul 29, 2019 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
    Commented Jul 29, 2019 at 18:46
  • 1
    \$\begingroup\$ @dfhwze Just edited my post to provide a direct link to that method. \$\endgroup\$ Commented Jul 29, 2019 at 18:55
  • 1
    \$\begingroup\$ @dfhwze Edited again to include those \$\endgroup\$ Commented Jul 29, 2019 at 19:33

2 Answers 2

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
\$\endgroup\$
1
  • \$\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\$ Commented Feb 20, 2020 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.
\$\endgroup\$
1
  • \$\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\$ Commented Aug 2, 2019 at 12:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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