Java: Method/Constructor argument validation. Also should there be multiple validation calls?

Question

I have a question about validating method/constructor arguments.

Let´s say I have a class BankAccount. This class performs some basic validation in the constructor (Note: that´s not how I would implement such a class, it´s better to use polymorphism and interface implementations for providing different types of bank accounts. Here an enum BankAccount.Type is used).

BankAccount(Type type, double balance, String ownerName, String accountNumber) {
    // Make sure no invalid arguments are passed.
    if (type == null) {
        throw new IllegalArgumentException("'type' may not be null!");
    }
    if (balance < type.getBalanceLimit()) {
        throw new IllegalArgumentException("initial 'balance' may not be less than the accounts balance limit!");
    }
    if (ownerName == null || ownerName.trim().isEmpty()) {
        throw new IllegalArgumentException("'ownerName' may not be null or empty!");
    }
    if (!type.requiresOwner()) {
        throw new IllegalStateException("Cannot create an account with an owner if that account type does not require an owner!");
    }
    if (accountNumber == null || accountNumber.trim().isEmpty()) {
        throw new IllegalArgumentException("'accountNumber' may not be null or empty!");
    }

    this.type = type;
    this.balance = balance;
    this.ownerName = ownerName;
    this.accountNumber = accountNumber;
}

First of all: Is this the way to go when validating constructor arguments or is it preferable to set all the fields first and then call a method validateState() which checks if the constructed state is valid and else throws an exception. Something like this:

BankAccount(Type type, double balance, String ownerName, String accountNumber) {
        this.type = type;
        this.balance = balance;
        this.ownerName = ownerName;
        this.accountNumber = accountNumber;

        validateState();
    } 

Which one of the above solutions is preferable (in most situations)?

The 2nd part of my question is about repeated validation. Let´s say I have a class Bank. This class has a method createGiroAccount(Account.Type type, String ownerName)

/**
         * Creates a new {@link Account} of the specified {@link Account.Type} for the given owner.
         * Note that the {@link Account.Type} must be a giro account type.
         *
         * @param type      the type of the new account which must be one of the available giro options, not null
         * @param ownerName the owner´s name who owns the new account, not null or empty
         */
        public void createGiroAccount(BankAccount.Type type, String ownerName) {
            // Really basic validation
            if (type == null || !type.isGiro()) {
                throw new IllegalArgumentException("'type' may not be null and must be of type giro!");
            }
            if (ownerName == null || ownerName.trim().isEmpty()) {
                throw new IllegalArgumentException("'ownerName' may not be null or empty");
            }

            String accountNumber = generateUniqueAccountNumber();
            accounts.add(new BankAccount(type, ownerName, accountNumber));
        }

My question: Should there too be a validation of arguments, even if the BankAccount validates the arguments/state anyway?

Answer 1

Having the validation within the constructor or in a dedicated method?

Both solutions are okay.

The trick is that if you add a lot of validation code into the constructor, mixed with the proper constructors code, it could be harder to maintain.

By the way if you choose to have a dedicated method validateState(); you should call it before doing anything into the contructor, it's useless to process code and later figure that one of your parameters is not valid and throw an exception....

If you want to have a clean validation you can use the javax.validation package. see this

You can then create your proper annotation an annotated your constructor parameters.

BankAccount(Type type, @ValidBalance double balance, @NotEmpty String ownerName, @NotEmpty String accountNumber) {

Where @ValidBalance will be a custom validation.

Answer 2

Lots of ways to do it, and none of them "right". In real-life systems, you'd have things like DB connection frameworks and REST APIs which assume you'll be using classes that don't have any arguments in the constructor at all.

So what you would probably have is an interface Account that can't have any validation anyway.

interface BackAccount {
    AccountType getAccountType();
    BigDecimal getBalance();
    Person getOwner();
}

Note that the owner is of a different class than String; this allows to assume that if you ever have a BankAccount, you won't need to validate the owner because Person validation happens when you create that object. You'd probably have a validation framework with something like

interface Validator<T> {
    boolean isValid(T toValidate);
}

which would allow you to implement validation for Persons, BankAccounts etc. In your case, you'd have a

class TypeBalanceValidator<BankAccount> implements Validator<BankAccount> {
    public boolean isValid(BankAccount account) {
        return account.getBalance().compareTo(getMinBalanceForType(account.getType())) > 0;
    }
}

As for creating the BankAccount, apart from automatically reading from a database, you'd probably have a Service which provides a factory method, not unlike yours, but with something like

interface AccountService {
    BankAccount createGiroAccount(Person p);
}

Why would you even pass the type to a method that only allows AccountType.GIRO? Anyway, this is where you'd create BankAccounts and where the validation would take place.

Answer 3

Is this the way to go when validating constructor arguments or is it preferable to set all the fields first and then call a method validateState()

You can create custom annotation and to check the validity of the parameters if you don't want to write repeating code.

The 2nd part of my question is about repeated validation.

If you have only one constructor then check is not needed in createGiroAccount. Otherwise you do.