3
\$\begingroup\$

I have this simple builder here:

public class User {

    private final String name;
    private final String lastname;
    private final String phone;

    private User(Builder builder) {
        this.name = builder.name;
        this.lastname = builder.lastname;
        this.phone = builder.phone;
    }

    public String getName() {
        return name;
    }

    public String getLastname() {
        return lastname;
    }

    public String getPhone() {
        return phone;
    }


    public static class Builder {
        private String name;
        private String lastname;
        private String phone = "";

        public Builder(String name, String lastname) {
            name(name);
            lastname(lastname);
        }

        private Builder name(String name) {
            this.name = Optional.ofNullable(name).orElseThrow(() -> new IllegalArgumentException("Name must not be null"));
            return this;
        }

        private Builder lastname(String lastname) {
            this.lastname = Optional.ofNullable(lastname).orElseThrow(() -> new IllegalArgumentException("Last name must not be null"));
            return this;
        }

        public Builder phone(String phone) {
            this.phone = Optional.ofNullable(phone).orElse("");
            return this;
        }

        public User build() {
            return new User(this);
        }

    }
}

This code works as expected but I have some doubts. My question is about private methods in builder (name and lastname). Is it allowed in a world of a clean code? And also about this Optionals, are they well written? I just started using this functionality so I don't know for sure if I am right here.

\$\endgroup\$
  • 1
    \$\begingroup\$ There is no point for name and lastname methods to return anything. Also those methods can be replaced by simply using java.util.Objects.requireNonNull() \$\endgroup\$ – user158037 Feb 2 '18 at 12:16
1
\$\begingroup\$

My understanding from a builder is a bit different. In my opinition, the User should not have a dependency to the Builder (you have a bidirectional dependency which is usually bad anyway). With that said, use the User as member variable within your builder:

public class UserBuilder {
  private User user = new User();

  public UserBuilder withUsername(String username) {
    user.setUsername(username);
  }

  public User build() {
    return user;
  }
}

I also prefer a verb in my method names (they should have, actally, but in that case I think it's okayish), that's why I use 'with' as prefix; And I explicitly do not use not 'set' as prefix, to distinguish it from the setter of the User type.

If you do it like that, the 'actual building' of the user is within the builder and not in the User - and the User type is kept quite stupid.

It might seem a bit strange, that the 'build' method doesn't actually build, but non-persistent built types are often only used for unit testing. The sweet part about builders, you can expand them to persist your built types or other fancy requirements.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.