System for registering people

For a small administrative program, I have to be able to register people and their data. But, files are sometimes created through a phone call "on the fly" and then later certain data is added to the file to complete it.

For this reason I started making my Person class with different constructors and I was chaining them, until I came to the conclusion that I probably had a bad design because each constructor was only adding one more parameter.

After some research, I implemented the Builder pattern (even found a nice example that used a Person class as example). But I am not sure if this is the correct approach for my situation. As I read (or understood), an object created through a Builder pattern is immutable. And as I said, data might sometimes be added later after a Person has been created already (phone number or e-mail or address, etc..).

This means I am changing my object, correct? Which is wrong to do.

Also, People have an address, but people can always move to a new location.
For this reason, I have an editAddress() method in my Address class.
This also means I am changing my object after it's been created, no?

And I plan to make this Person class my base class for 2 other types of Person; a child and a parent. Will my builder pattern still be useful, then?

My 3 questions are:

• Is it ok to add data after the object has been created? (if not, how should I approach this?)

• Is it ok to have methods in underlying classes that change the data? (if not, how should I approach this?)

• Will this still prove useful once I start with my sub classes; child and parent?

My first code with the chained constructors

```
public class Person {

private String name;
private String familyName;
private String dateOfBirth;
private Address address;
private String phoneNumber;
private String eMail;

public Person(String name, String familyName) {
this.name = name;
this.familyName = familyName;
}

publi

For this reason i started making my Person class with different constructors

As long as everybody can remember what the constructors do, it may be fine. Something like C++-like arguments with default values.

public Person(String name, // no valid Java
    String familyName=null,
    String dateOfBirth=null,
    Address address=null,
    String phoneNumber=null,
    String eMail=null)

This exactly corresponds with your bunch of constructors and IMHO it goes too far. Close your eyes and tell me if the email comes after or before the phone number. What about adding a second phone number?


bad design because each constructor was only adding one more parameter.

This is just because of Java lacking the optional arguments, that's fine. The problem is too many arguments and most of them being strings. too error-prone.


But i am not sure if this is the correct approach for my situation.

It's not as you don't need it. Immutables are fantastic, but even if you dropped your setters, there are still too many stupid tools around which can't work with an immutable Person.


So this means i am changing my object, correct?

Correct. You could avoid it by creating a new instance containing modified data. That's common and fine, but you most probably don't need it. Entities are always mutable, I'd stick with it.


And.. i plan to make this Person class my base class for 2 other types of Person; a child and a parent.

Consider avoiding inheritance (maybe delegation?). And what about a child becoming a parent?


Will my builder pattern still be useful then?

No.... with delegation it'd better.

Finally to the code

My first code with the chained constructors

Let's call it "constructor triangle". It may look nice, but it won't make your live any easier. I'd stick with the first constructor.

My code with the builder pattern

private Date dateOfBirth;

Date is the worst class ever. Consider using new Java 8 classes or Joda-Time.

public Person(PersonBuilder builder) {
    ...
    this.dateOfBirth = builder.dateOfBirth;
    this.address = builder.address;
    ...
}

Whenever you store a mutable object in your class, it means that it can be changed later without accessing the person. You should clone them.

private Address address = new Address("Unknown","00","0000","Unknown");

"Unknown", "00" and similar is what you want to present to the user. As a programmer you need something what can be tested easily, i.e. the empty string.

Summary

Your code is mostly fine, but as you see, builder for a mutable object makes little sense. What you need is fluent setter, so you can write

Person john = new Person("John", "Doe")
    .phoneNumber("123456")
    .email("john@doe")
    ...
    .address(address)
    .whatever("whatever");

That's much less error-prone that the length constructor triangle and pretty easy to use.

You can flatten the address into street, houseNr, areaCode, and city, or use an object, but then either Address should be immutable or the setter should clone it. Or flatten it completely and create no Address class at all (not nice, but easy).

I'd suggest to use project Lombok and then all you need is

@Accessors(chain=true, fluent=true) @Getter @Setter
public class Person {
    // This is not error-prone and saves you a few chars
    public Person(String name, String familyName) {
        this.name = name;
        this.familyName = familyName;
    }

    private String name;
    private String familyName;
    private SomeImmutableDate dateOfBirth;
    private String street, houseNr, areaCode, city;
    private String phoneNumber;
    private String eMail;
}

Immutable version

And when you need an immutable object, I'd suggest

@Accessors(chain=true, fluent=true) @Getter @Builder
public class Person {
    private final String name;
    ...
}

StackExchange Code Review Q#86674, answer score: 6