Return to Answer

improved explaination a bit

Source Link

edited Feb 15, 2016 at 9:51

Set<DogValidationException> exceptions = new HashSet<DogValidationException>();
boolean retry;
do {
    retry = false;
    try {
        dog.validate(exceptions);
    } catch (DogValidationException e) {
        exceptions.add(e);
        retry = true;
    }
} while (retry); 

if(exceptions.isEmpty()) {
    //dogDAO.beginTransaction();
 commit transaction  dogDAO.save(dog);
    dogDAO.commitAndCloseTransaction();
} else {
    // notify user to fix the problems
}

The problem here is that the validation process, to get all errors, would require to skip already found exceptions. The above could work, but this is a clear misuse of exceptions. The kind of validation you were asked for should take place before the database is touched. So there is no need to roll back anything. And, the results of the validation are likely to be validation errors (hopefully zero, though).

Set<Problem> validationResults = dog.validate();
if(validationResults.isEmpty()) {
    //dogDAO.beginTransaction();
 commit transaction  dogDAO.save(dog);
    dogDAO.commitAndCloseTransaction();
} else {
    // notify user to fix the problems
}

Why? There are tons of reasons, and the most reasons have been pointed out in the other responses. To put it simple: It is much simpler to read and understand by others. Second, do you want to show the user stack traces to explain him he set up his dog wrongly?

If, during the commit in the second example, still an error arises, even though your validator validated the dog with zero issues, then throwing an exception is the right thing. Like: No database connection, the database entry has been modified by someone else meanwhile, or that like.

Set<DogValidationException> exceptions = new HashSet<DogValidationException>();
boolean retry;
do {
    retry = false;
    try {
        dog.validate(exceptions);
    } catch (DogValidationException e) {
        exceptions.add(e);
        retry = true;
    }
} while (retry);
if(exceptions.isEmpty()) {
    // commit transaction
} else {
    // notify user to fix the problems
}

The problem here is that the validation process, to get all errors, would require to skip already found exceptions. The above could work, but this is a clear misuse of exceptions.

Set<Problem> validationResults = dog.validate();
if(validationResults.isEmpty()) {
    // commit transaction
} else {
    // notify user to fix the problems
}

Set<DogValidationException> exceptions = new HashSet<DogValidationException>();
boolean retry;
do {
    retry = false;
    try {
        dog.validate(exceptions);
    } catch (DogValidationException e) {
        exceptions.add(e);
        retry = true;
    }
} while (retry); 

if(exceptions.isEmpty()) {
    dogDAO.beginTransaction();
    dogDAO.save(dog);
    dogDAO.commitAndCloseTransaction();
} else {
    // notify user to fix the problems
}

Set<Problem> validationResults = dog.validate();
if(validationResults.isEmpty()) {
    dogDAO.beginTransaction();
    dogDAO.save(dog);
    dogDAO.commitAndCloseTransaction();
} else {
    // notify user to fix the problems
}

Source Link

created Feb 15, 2016 at 9:31

Matthias Ronge

There may be a requirement to report on every error we can find in the input, not just the first.

This is why you cannot throw an exception here. An exception immediately interrupts the validation process. So there would be much work-around to get this done.

A bad example:

Validation method for Dog class using exceptions:

void validate(Set<DogValidationException> previousExceptions) {
    if (!DOG_NAME_PATTERN.matcher(this.name).matches()) {
        DogValidationException disallowedName = new DogValidationException(Problem.DISALLOWED_DOG_NAME);
        if (!previousExceptions.contains(disallowedName)){
            throw disallowedName;
        }
    }
    if (this.legs < 4) {
        DogValidationException invalidDog = new DogValidationException(Problem.LITERALLY_INVALID_DOG);
        if (!previousExceptions.contains(invalidDog)){
            throw invalidDog;
        }
    }
    // etc.
}

How to call it:

Set<DogValidationException> exceptions = new HashSet<DogValidationException>();
boolean retry;
do {
    retry = false;
    try {
        dog.validate(exceptions);
    } catch (DogValidationException e) {
        exceptions.add(e);
        retry = true;
    }
} while (retry);
if(exceptions.isEmpty()) {
    // commit transaction
} else {
    // notify user to fix the problems
}

The problem here is that the validation process, to get all errors, would require to skip already found exceptions. The above could work, but this is a clear misuse of exceptions.

The better approach is:

Method call:

Set<Problem> validationResults = dog.validate();
if(validationResults.isEmpty()) {
    // commit transaction
} else {
    // notify user to fix the problems
}

Validation method:

Set<Problem> validate() {
    Set<Problem> result = new HashSet<Problem>();
    if(!DOG_NAME_PATTERN.matcher(this.name).matches()) {
        result.add(Problem.DISALLOWED_DOG_NAME);
    }
    if(this.legs < 4) {
        result.add(Problem.LITERALLY_INVALID_DOG);
    }
    // etc.
    return result;
}