When and how to check input parameters

Question

My SmsRecipientDetails class constructor accepts String value as recipient phone number.

I would like to accept number with spaces:

assertDoesNotThrow(() -> new SmsRecipientDetails("123456789 "));
assertDoesNotThrow(() -> new SmsRecipientDetails(" 123456789 "));
assertDoesNotThrow(() -> new SmsRecipientDetails(" 123456789"));

Current constructor that does not accept phone number with spaces:

public SmsRecipientDetails(final String phoneNumber) {
   AssertUtils.notNull(phoneNumber, "phoneNumber");
   AssertUtils.validState(phoneNumber.matches("^[0-9]{9}$"), "Phone number is not valid");
   this.phoneNumber = phoneNumber;
}

Q1: Where and how to trim input phoneNumber parameter? Is this implementation with one temp value correct? I am not sure when there will be more input parameters because of clarity.

public SmsRecipientDetails(final String phoneNumber) {

    AssertUtils.notNull(phoneNumber, "phoneNumber"); // throws exception when phoneNumber is null

    final String toCheck = phoneNumber.trim();
    AssertUtils.validState(toCheck.matches("^[0-9]{9}$"), "Phone number is not valid");

    this.phoneNumber = toCheck;

  }

Q2: Is it good idea to allow not correct values and preprocess it in constructor?

Answer 1

I think this is a very subjective question which goes into what is the correct amount of validation you should have and where should it be positioned.

Example: If you have a flight company you want your purchase to go. You dont want to stop the purchase and if there is an error you resolve it at later phase via different channels - helpdesk service, reservation beurau and so on.

If you are a bank. You want your bank transactions to be 100% correct.

So when and how much is very much a question of what software you are writing.

With regards of Q2 again no definitive answer on where. Placing the validation in the constructor is an immediate validation which in certain systems is OK in certain systems not OK. You may have also a validation that is happening post factum. Like e-commerce site will first allow the purchase to go, and possibly then validate it.

There is aways this question how consistent and correct the data should be. No definitive answer, aways depends on the case.

Answer 2

Validation and throwing exceptions from a constructor is subject to debate, see for example this discussion here on stackexchange. Personally, I avoid validating and throwing exceptions from constructors as much as possible.

So, I would not accept and/or validate a phonenumber in a constructor. The better way I think is to implement a Parse(string phoneNumber) or a TryParse(string phoneNumber, out SmsRecipientDetails result), just like the DateTime struct does in C#.

Answer 3

You do not appear to be validating input. This looks more akin to sanitizing or massaging input. I see nothing wrong with stripping irrelevant formatting characters in the phone number before validating. After all, what's the difference between 1 555-123-4567 versus +1 555-123-4567 or (555) 123-4567 or 555-123-4567? Who cares if the phone number was entered with dots: 555.123.4567, or colons, or — whatever. Phone numbers are basically digits. You can stretch that to letters as well.

Trim the whitespace from the beginning and end of the string, then validate.

---

And yes, I understand some of the phone numbers in my answer lack a country code. That can be inferred from the user's locale if they have a profile in the application, or if they have chosen a country of origin.