1

SO community.

I have this small piece of code that I am not sure if it's not filled with bad practices. Basically the email message that i am trying to validate holds some data that i am interested in later on.

I wanted to ask if it's good practice to validate passed parameters in the constuctor and throw some kind of exception afterwards.

How would YOU handle this?

Thank you, Bob.

public EmailSubjectValidator(EmailConfig emailConfig, Message msg) {
    this.emailConfig = emailConfig;
    if (msg == null || msg.isExpunged()) {
        throw new NullPointerException("Error: EmailSubjectValidator found a message that is either null or was already removed from the server. ");
    }
    this.msg = msg;
}
Bobzone
  • 276
  • 2
  • 12

1 Answers1

2

In general, validating parameters in constructor is not a bad practice, it's always depends on what you are actually implementing.

In your particular case I recommend you the following:

1) If you want to validate data in constructor, don't throw NullPointerException, but rather IllegalArgumentException;

2) As your class is named EmailSubjectValidator, I would not validate the data in constructor. You can use constructor for initialising the data instead. It would be much better to have validate() method which will do this;

3) Validating the data is a common thing to do in a lot of cases, so it may be a good idea to create a Validator interface with single validate() method for making validation;

yyunikov
  • 5,719
  • 2
  • 43
  • 78
  • The Validator class is a really good suggestion. – J. Pichardo Oct 20 '16 at 17:23
  • Thank you for the answer - your suggestions make me ask another questions though: – Bobzone Oct 20 '16 at 17:36
  • Is it better to have an Validator interface and then two other classes: EmailSubjectValidator and EmailAttachmentsValidator or rather one interface Validator and one more general class EmailValidator with two methods that validate email subject and attachments separately? – Bobzone Oct 20 '16 at 17:37
  • I would go with the second option, one EmailValidator class. But in case if you have a lot of code and logic for validation, then it makes sense separate them to subject and attachments validator and then finally combining them both in EmailValidator class, so it will be easier to use. – yyunikov Oct 20 '16 at 17:41