1

I have a Java class Validator. It validates some file. Here it is in pseudo code:

public class Validator {
    Collection<ValidationMessage> validationMessages;

    class ValidationMessage {
    }

    enum MessageType {
    }

    public validate(lines) throws ValidationException {
        for (line : lines) {
            validateCriteriaA(line)
            validateCriteriaB(line)
            ... 
            validateCriteriaF(line)
        }
        if (...)
            throw new ValidationException();
    }

    private validateCriteriaA(Line line) {
        if (...) {
            validationMessages.add("criteria failed")
        }
    }
    private validateCriteriaB(Line line)
    private validateCriteriaC(Line line)
    private validateCriteriaD(Line line)
    private validateCriteriaE(Line line)
    private validateCriteriaF(Line line)
    private helperMethodX
    private helperMethodY
    private helperMethodZ
}

Each method has no more than 15 lines. The total length of this class is ~300 lines. Do you think it is a God object or some other bad smelling class? Is it some kind of anti-pattern? If you think it needs to be redesigned - please tell me how.

Jason D
  • 8,023
  • 10
  • 33
  • 39
Oleksii Volynskyi
  • 1,317
  • 1
  • 8
  • 5
  • This is probably a better question on [http://codereview.stackexchange.com/](http://codereview.stackexchange.com/) – brichins Aug 11 '15 at 19:16

1 Answers1

3

I wouldn't call it a god class, but I would call it a violation of the open-closed principle.

A problem I see is that this Validator doesn't scale. With only half a dozen Criteria, its size is reasonable; but to add Criteria in the future, you must increase this class each time. Furthermore, this Validator cannot change its logic at runtime, for example to skip or reorder its Criteria.

The Criteria could be separated out into individual strategy classes to solve all of these problems.

jaco0646
  • 15,303
  • 7
  • 59
  • 83