From @pepijno 's answer:
What I've seen the most when it comes to builder patterns is that the object (in your case Animal) will get constructed in the build method of the builder. It is preferred to create instances of objects with all the data that the object should have, eg not creating invalid states of an object.
That is true. However his following code contradicts this.
public Animal(){ }
public String getAnimalName() {
return animalName;
}
public int getAnimalAge() {
return animalAge;
}
So now I am free to create animal of age 0 and empty name? I know OP's code did not check for this. But if there were any checks like that (and they probably should), you are now free to create Animal with invalid state.
The way you should interpret the above quote is like this:
public class Animal
{
private String animalName;
private int animalAge;
public Animal(String name, int age){
if (name.length() == 0) {
throw new Exception('Name cannot be empty');
}
if (age < 0) {
throw new Exception('Age cannot be negative');
}
animalName = name;
animalAge = age;
}
public String getAnimalName() {
return animalName;
}
public int getAnimalAge() {
return animalAge;
}
}
class AnimalBuilder
{
private String animalName;
private int animalAge;
public AnimalBuilder(){}
public AnimalBuilder setAnimalName(String name) {
animalName = name;
return this;
}
public AnimalBuilder setAnimalAge(int age) {
animalAge = age;
return this;
}
public Animal build() {
return new Animal(animalName, animalAge);
}
}
Now, notice a few things.
AnimalBuilder is now ouside Animal class. This removes the circular dependency of the two classes. Builder knows the target Animal class, but the Animal is unaware of its building steps. At the same time, the Animal can be constructed without a builder and it cannot end up in an invalid state. This also explains why passing the builder to the animal constructor is bad, it would again introduce the circular coupling of the two classes (and also reminds service locator anti pattern). Same reason why in pepijno's answer, putting builder() method on the Animal class, is bad.
There might be more specific exceptions to use, but I don't know Java.
Also builder's methods should really be called set*, not get*, nor with* (as in pepijno's answer). The "withers" are usualy implemented in an immutable way:
class MyClass
{
private String myName;
private String other;
public MyClass(String name, String x) {myName = name; other = x;}
public MyClass withName(String name)
{
return new MyClass(name, other);
}
}
As for the approach i noticed in comments
https://codereview.stackexchange.com/a/127509/221293
Yes this is possible, but an interface should be created with only the getters and implemented by the Animal class to prevent consumers of constructed Animals from modifying it.
interface IAnimal
{
public String getAnimalName();
public int getAnimalAge();
}
public class Animal : IAnimal
{
private String animalName;
private int animalAge;
public Animal(String name, int age){
setAnimaName(name);
setAnimalAge(age);
}
public String getAnimalName() {
return animalName;
}
public void setAnimalName(String name) {
if (name.length() == 0) {
throw new Exception('Name cannot be empty');
}
animaName = name;
}
public int getAnimalAge() {
return animalAge;
}
public void setAnimalAge(int age) {
if (age < 0) {
throw new Exception('Age cannot be negative');
}
animalAge = age;
}
}
And the builder should be declared to return that interface and not the class.
class AnimalBuilder
{
...
public IAnimal build() {...}
...
}
But the Animal class should remain only constructible into a valid state, but the builder requires it to have default constructor (which restricts the usage to objects with default constructors, ie. where empty name is allowed).
And setting any property of the class after construction, must not corrupt the object. This to prevent bugs if someone decides to create the Animal directly, not using a builder, thus knowing its setters.
And this is why the approach from the mentioned SO post has restricted usage. It is also half the way between circular coupling and decoupling the two. Because although the Animal still does not know AnimalBuilder, it provides setters in expectation of existance of a builder that could use those setters (or it expects consumer to modify it, but that would be a completly different scenario than in your (OP) question).
It is also unsuitable when there are validations that depend on multiple properties (ie. if name is dog, his age not only must be non-negative, it cannot be more then 20 - ignore the nonsense of the example :D), because setting the two separately with distinct setters may lead to different behaviour depending on the order in which the setters are called.
The approach I proposed (high) above is generic and always applicable. The amount of repeating is about the same, IMO.
