System for registering people

Question

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:

1. Is it ok to add data after the object has been created? (if not, how should I approach this?)
2. Is it ok to have methods in underlying classes that change the data? (if not, how should I approach this?)
3. 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;
    }

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

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

    public Person(String name, String familyName, String dateOfBirth, Address address, String phoneNumber) {
        this(name, familyName, dateOfBirth, address);
        this.phoneNumber = phoneNumber;
    }

    public Person(String name, String familyName, String dateOfBirth, Address address, String phoneNumber, String eMail) {
        this(name,familyName,dateOfBirth,address,phoneNumber);
        this.eMail = eMail;
    }

}

My code with the builder pattern

public class Person {

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

    public Person(PersonBuilder builder) {
        this.name = builder.name;
        this.familyName = builder.familyName;
        this.dateOfBirth = builder.dateOfBirth;
        this.address = builder.address;
        this.phoneNumber = builder.phoneNumber;
        this.eMail = builder.eMail;
    }

    @Override
    public String toString() {
        return String.format("%s %s\n%tD\n%s\nTel:%s\nEmail:%s",this.name,this.familyName,this.dateOfBirth,this.address,this.phoneNumber,this.eMail);
    }


    public static class PersonBuilder {

        private final String name;
        private final String familyName;
        private Date dateOfBirth = new Date();
        private Address address = new Address("Unknown","00","0000","Unknown");
        private String phoneNumber = "0000/00 00 00";
        private String eMail = "[email protected]";

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

        public PersonBuilder dateofBirth(Date dateOfBirth) {
            this.dateOfBirth = dateOfBirth;
            return this;
        }

        public PersonBuilder address(Address address) {
            this.address = address;
            return this;
        }

        public PersonBuilder phoneNumber(String phoneNumber) {
            this.phoneNumber = phoneNumber;
            return this;
        }

        public PersonBuilder eMail(String eMail) {
            this.eMail = eMail;
            return this;
        }

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

    }

}

My editAddress method

public void editAddress(Address address){
        this.street = address.street;
        this.houseNr = address.houseNr;
        this.areaCode = address.areaCode;
        this.city = address.city;
    }

Answer 1

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;
    ...
}

Answer 2

There's actually a small but significant problem with your 'chained constructors' approach... If you prefix your fields with the final modifier, you'll find that every constructor has compilation errors. That's because you will have additional arguments that are not defined with the 'simpler' constructors, and the compiler helpfully prompts you that fields may already been initialized in the 'advanced' constructors (which will be an errors too).

As pointed out in my answer for another question, this cascading of constructors will only work if you invert the 'flow' (I'm just using null as the default arguments, for simplicity):

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

public Person(String name, String familyName, String dateOfBirth) {
    this(name, familyName, dateOfBirth, null);
}

public Person(String name, String familyName, String dateOfBirth, Address address) {
    this(name, familyName, dateOfBirth, address, null);
}

public Person(String name, String familyName, String dateOfBirth, Address address, String phoneNumber) {
    this(name, familyName, dateOfBirth, address, phoneNumber, null);
}

public Person(String name, String familyName, String dateOfBirth, Address address, String phoneNumber, String eMail) {
    this.name = name;
    this.familyName = familyName;
    this.dateOfBirth = dateOfBirth;
    this.address = address;
    this.phoneNumber = phoneNumber;
    this.eMail = eMail;
}

Mutable vs immutable classes

The long answer is that, it depends. For basic programming tasks within a single-threaded context, you can't go very wrong with mutable classes, so there's that. For multi-threaded/concurrent usage, the general suggestion is to default to immutable classes, so that an object's state is always consistent when reading, and thus a good defensive approach to invalid state, data corruption, unexpected concurrent modification etc.

A further point you can consider in answering this question is how you are dealing with the object's history and persistence at the end of the day. For example, if you do not need a person's address history for the application's lifecycle, then I certainly see no issue with a simple editAddress() method where the details are updated within a Person instance. Alternatively, if you are persisting a Person object as a single line in a CSV file, then this approach makes sense too - it gets the job done without introducing too much boilerplate template code to handle different addresses or output formats, for example.

In a more complex structure, such as having a normalized database with a [1...N] relationship between your Person and Address entities, then the you should probably be considering supplementary classes such as PersonAddresses that handily maps a Person class (on an id field?) to one more addresses. This is one of the cases where you will need something more advanced implementation than a simple copying of fields within your editAddress() method. It also depends on your use case - do you need to handle home and work addresses?

It's a happy family

Parents and children are still Person instances after all, so you probably shouldn't need to use inheritance (pun intended) to implement something like Parent and Child classes, if that is what you are thinking... For the purposes of your registration usage, you probably have:

• A standalone patient with no parents or children in the (same) system
• A patient whose parents' details are in the system
• A patient whose children's details are in the system

Therefore, you might have fields like List<Person> parents and List<Person> children, and your builder pattern needs to take care of them. Of course, you will need further validation of adding/removing parents and children, which will be left as an exercise for the reader... :)