Builder pattern & failling

Question

Consider the following class:

@Getter
public class EmailVO {
    private final Long id;
    private final String firstName;
    private final String email;
    private final String address;

    @Slf4j
    @Component
    @Scope("prototype")
    public static class Builder {
        private Lead lead;

        private Long id;
        private String firstName;
        private String email;
        private String address;

        public Builder fromLead(Lead lead) {
            this.lead = lead;
            return this;
        }

        public EmailVO build() {
            if (lead == null) {
                log.error("Failed to build EmailVO: lead was not initialized");
                return new EmailVO(this);
            }

            User user = lead.getUser();
            id = user.getId();
            firstName = user.getFirstName();
            email = user.getEmail();
            address = user.getAddress();

            return new EmailVO(this);
        }
    }

    private EmailVO(Builder builder) {
        id = builder.id;
        firstName = builder.firstName;
        email = builder.email;
        address = builder.address;

        if (id == null ||
            firstName == null ||
            email == null ||
            address == null)
        {
            throw new IllegalStateException(); // Maybe some ohter Unchecked Exception would be better
        }
    }
}

As far as I know, this would be a proper VO class implementation which only permits to build new instances from it's builder, which also follows adequately the builder pattern (correct me if I'm wrong).

From a SOLID perspective, this code is fine as the builder single responsibility is aggregating data to build EmailVO and it's constructor will take care of letting only valid instances of it to come to life.

Now, if you care for code cluttering and readability (imagine a much bigger VO for the case), the builder could fail when one tries to build without required parameters initialized instead of letting the object's constructor to fail which can potentially remove many null checks inside the constructor. In the example code, if lead field is null this one validation could throw an exception instead of letting EmailVO's constructor to check for integrity despite being the constructor's responsibility.

Would it be OK to remove the validation from the constructor of EmailVO and letting the builder to take care of it? Consider that in this case the constructor is private making it invisible to the outside of this class.

This seems like going against SOLID despite that if the builder is not validating required parameters it can fail in it's one responsibility which is to aggregate required data to build an EmailVO instance.

Yet, an idea that crossed my mind is having a flag as member field of the EmailVO.Builder class to signalize if it succeed or not in aggregating the required parameters and then EmailVO's constructor could only check (and trust) this flag.

Answer 1

Considering that your EmailVO is strictly for holding values, I suggest that you minimize the login in the setters and constructor (variation of the @Christopher Schneider comment).

If you want validation, you might add a validation method to the EmailVO object.

About the "build with bad values" question, add that validation to the builder.

The EmailVO validation might include something like "is this a validly constructed email address".

Also, don't pass the builder to the EmailVO object, just have a package access constructor that is used by the builder. Be sure that the builder is in the same package as the EmailVO object.