Collect all violations while validating NumberSchema

[adyach]

It is stated that Starting from version 1.1.0 the validator collects every schema violations (instead of failing immediately on the first one), but for some reason it was not like that for NumberSchema. This PR addressed this minor issue by introducing instance field with the list of violations.

[coveralls]

Coverage increased (+0.02%) to 91.757% when pulling f19e691 on adyach:gather-valations-numberschema into 98aeeab on everit-org:master.

[erosb]

This shouldn't be a member field, because this way the NumberSchema instance won't be thread-safe. I suggest handling it as a local variable in the validate() method and change the signatures of the check* methods to return an Optional<ValidationException> (or a List<ValidationException> if there are possible multiple errors checked).

You can work from StringSchema as an example.

[adyach]

@erosb thank you for feedback, I did not know it should be thread safe. do you think that with the approach like StringSchema you generate to way many unnecessary objects? For example, you create 3 ArrayLists even though it is possible they will be never used and I did not count call to addAll, which does copy of arrays.

[erosb]

Correctness first. I wouldn't go into too much micro-optimizations here. At least not as long as it isn't verified with a perf. test that this is a bottleneck.
This entire aspect should be refactored at some time anyway (I would like to make it configurable to collect all errors or fail immediately with the first one), so I really don't see the point in putting too much effort into reducing the GC load here.

[adyach]

1. create list before hand and pass it as argument to check* (save 2 list creations)
2. (A bit ugly) create function:

    private List<ValidationException> getList(List<ValidationException> validationExceptions,
                                              ValidationException validationException) {
        if (validationExceptions == null) {
            validationExceptions = new ArrayList<>();
        }
        validationExceptions.add(validationException);
        return validationExceptions;
    }

Once you encountered an error in the check*, you call that function by passing listA and then assign the result of that function back to the listA, and so forth.

    private List<ValidationException> checkMaximum(final double subject) {
        List<ValidationException> validationExceptions = null;
        if (maximum != null) {
            if (exclusiveMaximum && maximum.doubleValue() <= subject) {
                validationExceptions = getList(validationExceptions, failure(subject + " is not less than " + maximum, "exclusiveMaximum"));
            } else if (maximum.doubleValue() < subject) {
                validationExceptions = getList(validationExceptions, failure(subject + " is not less or equal to " + maximum, "maximum"));
            }
        }
        if (exclusiveMaximumLimit != null) {
            if (subject >= exclusiveMaximumLimit.doubleValue()) {
                validationExceptions = getList(validationExceptions, failure(format("is not less than " + exclusiveMaximumLimit),"exclusiveMaximum"));
            }
        }
        return validationExceptions;
    }

[erosb]

If anything, then it makes more sense to optimize for the initial size of the list. new ArrayList<>() allocates space for 10 objects. In case a check* method can find at most 2 errors, you can create the list with new ArrayList<>(2) .

[adyach]

Okay, I see your point, I did it a little bit different from StringSchema. Please, check it out.

[erosb]

Thank you for raising this PR, this is definitely a bug. I added my concerns to the diff. Overall the change looks good, so I will merge it if you address my comments.

[adyach]

@erosb moved the instance field into the method to keep thread safety , please, check it out.

[coveralls]

Coverage increased (+0.01%) to 91.753% when pulling 4324392 on adyach:gather-valations-numberschema into 98aeeab on everit-org:master.

[erosb]

Looks good, thank you @adyach for your contribution.