validate first or try catch?

Question

Let's say I have a function that looks like this:

public void saveBooking(/* some inputs */) {
    //save into database
}

Before saving into database, I have to do various validations. What I can do in my main program is like this:

//do all the validations and do any necessary handling. Then...
saveBooking(/*inputs*/);

With this, I'm sure that all the data have to pass all the validations required before saving into database. However, this means that the function saveBooking() closely depends on the validation methods. Every time I want to call saveBooking(), I have to make sure that I don't forget to call the validations.

Alternatively, I can put all the validations inside the function itself so that all I should do is to call the method and everything is taken care of. However, in order to handle all the errors independently, I have to do make the function throw exceptions and catch in the main program. It should look something like this:

public void saveBooking(/* some inputs */) /* throws various exceptions */ {
    //various validations
    //save into database
}

//...and in the main program...
try{
    saveBooking(/*inputs*/);
}
catch(MyException1 e1){
    //do something
}
catch(MyException2 e2){
    //do something
}

This also means I have to create multiple exceptions on my own. The good thing is I don't have to worry what validations I have to put before hand.

With these, I'm not sure which one is the best code design. I personally prefer the first method which is more readable but it depends on each other too much and it's getting worse when I need to use it in many places.

Answer 1

Definitely the first option over the second. I consider the second to be an abuse of exceptions. Exceptions are meant for exceptional circumstances, and failing validation is not "exceptional."

Every time I want to call saveBooking(), I have to make sure that I don't forget to call the validations.

Put the validation logic into a separate method, and have saveBooking() call the validation method before it does anything else.

public List<ValidationError> validateBooking(/* args */)
{
    // as @Jared Farrish suggests, return a list of validation errors
}

public boolean saveBooking(/* args */)
{
    List<ValidationError> errors = validateBooking(/* args */);

    if (errors.size() != 0) return false; // validation failed

    // save to the database

    return true;
}

Answer 2

The checking should usually be carried out within the function itself so that there is no possibility of trying to save the data without first validating it. Without those checks within the function, you could find a client trying to save without validation and that's rarely a good thing.

But you're not restricted to using exceptions for this, you can simply return an error code of some sort to be checked by the caller. While I usually don't care whether errors are done by exceptions or return codes, there are some that may see this a an abuse of exceptions.

The validation code is probably still left as a separate function since your own code may wish to call it without doing the save as well. Something like (pseudo-code):

def validateStuff():
    if stuff is not valid:
        return WTF
    return OK

def saveBookings():
    rc = validateStuff()
    if rc != OK:
        return rc;
    save the information
    return OK

You'll still likely have exceptions caught by your call to saveBookings if only to handle I/O errors and the like, but it's not absolutely necessary: you could catch those exception within the function as well and translate them to a return code.

I tend to like one reporting method from each function so I (for example) don't have to try/catch and check return codes.

Answer 3

You are definitely doing the right thing by validating everything before insertion, you should also validate if everything that you are inserting will fit or meets the constraints in your database to avoid an unexpected SQLException, which you won't be expecting and will go all the way to the top.

I would recommend creating a custom exception with some attributes to describe the cause of the error, this way you only have to worry about catching one kind of exception.

Aditionally I would definitly put the validation inside the method, so it is always called.