Question

I am working on a java web application and I have a few questions regarding design.

Basically in its current version, it relies heavily on catching exceptions to determine the flow of control.

For instance in one of my spring service classes, I have the following method that checks whether an email given as a parameter exists in the database.

@Override
public boolean validateEmailAddressDoesNotExist(String accountEmailAddress) {
    try {
         return !dao.checkIfEmailAddressAlreadyExists(accountEmailAddress);
    } catch (NoResultException re) {
        log.error("NoResultException", re);
    } catch (RuntimeException re) {
        log.error("RuntimeException", re);
    }
    return true;
}

//from "dao" class
public boolean checkIfEmailAddressAlreadyExists(String accountEmailAddress) {
    return (loadAccountFromAccountEmailAddress(accountEmailAddress) == null ? false : true);
}

//also from "dao" class
public Account loadAccountFromAccountEmailAddress(String accountEmailAddress) {
    return entityManager.createNamedQuery("Account.findByEmailAddress", Account.class).setParameter("accountEmailAddress", accountEmailAddress).getSingleResult();
}

I suspect that my current design is probably wrong but I would be grateful to read your comments and opinion about it and to what extent you believe it is flawed.

Was it helpful?

Solution

Validation methods in your service model should not be catching exceptions. That's bad for a few reasons:

  • It's not an exceptional condition. "No results" is a common situation.

  • It indirectly couples your validation to the implementation of the framework's data-retrieval methods. To see why that's not good, imagine if your framework changes such that it now raises a EmptyResultSetException. You'd have to update all your validation methods. Yikes!

You can't necessarily help it if your underlying framework raises exceptions to indicate "no results", but you can certainly control what checkIfEmailAddressAlreadyExists does.

Change this method so that it returns true if the address exists, and false if it doesn't or if no results were found.

OTHER TIPS

The general rule-of-thumb is that exceptions are for "exceptional" conditions.

So if a data item might be there, or might reasonably be absent, then returning a boolean would be more normal. It also usually results in simpler, clearer code.

You can then save Exceptions for truly exceptional conditions such as network failures etc.

In some cases, 3rd party libraries might not give you any choice - if they throw exceptions then you have to deal with them!

I prefer to return a boolean from methods like checkIfEmailAddressAlreadyExists, and simply control the flow based on the return value and leave Exceptions for truly exceptional conditions, not being able to connect to the database for example.

I am not a Java programmer, never used it in fact.

But I do know that raising and catching exceptions is very expensive in C# world. So, controlling flow with them is very inefficient as opposed to checking things yourself and leaving exceptions for things you didn't think of as DNA said.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top