I am working on a Pokemon game at the moment, and am running into some design concerns. The easiest example is as follows:
Each Species
of Pokemon has several traits that are required before it is logically initialized. These include, but are not limited to:
id
: an English representation of thename
(usually just the name lowercase). Example:charizard
name
: the localized name of theSpecies
. Example:Charizard
in English,Rizardon
in Japanese, andDracaufeu
in French.primaryType
: the primary element of thisSpecies
. Example:Charizard
‘s primary type isFire
because it is a fire-breathing dragon.secondaryType
: (optional) eachSpecies
may also have a secondary type. Example:Charizard
‘s secondary type isFlying
, but it’s pre-evolution does not have wings, so it does not have asecondaryType
.null
is okay here.baseStats
: the base combat abilities of all members of aSpecies
evPointYield
: the type of experience gained after defeating a member of aSpecies
For brevity, I’ll stop there. There are at least 4 other fields that are required, but are not important for the example.
In this example, because all fields are required before a Species
is constructed (read: no Species
can possibly exist without all of these fields), a constructor would look something like
public Species(final String id, final String name, final Type primaryType,
final Type secondaryType, final StatSet baseStats, final StatSet evPointYield) {
// argument checking omitted for brevity
this.id = id;
this.name = name;
this.primaryType = primaryType;
this.secondaryType = secondaryType;
this.baseStats = baseStats;
this.evPointYield = evPointYield;
}
There is no way to logically group these into different objects to reduce the amount of parameters passed into the constructor.
My first attempt at a solution was to make a Builder
public class Species {
public static final class Builder {
// setters for all 6 fields
public Pokemon build() {
return new Pokemon(this);
}
}
public Species(final Species.Builder builder) {
// ensure builder is not null
// set fields from builder
}
}
But this would all users to build, while not calling any methods like so
Species species = new Species.Builder().build();
So, I modified the build method to include a validation method
public Species build() {
if (!stateIsValid()) {
// actual message is more descriptive, including which specific fields
// were missed
throw new IllegalStateException("not all fields were initialized before building");
}
return new Species(this);
}
private boolean stateIsValid() {
return
idIsValid() &&
nameIsValid() &&
primaryTypeIsValid() &&
secondaryTypeIsValid() &&
baseStatsAreValid() &&
evPointYieldIsValid();
}
private boolean idIsValid() {
// return it was set to a valid value
}
// other state checkers
I should also note that I am using Species
as more of a data type. Meaning, that the fields will be loaded from somewhere, and there are very few logical operations that would be necessary in this class. This class (sh/w)ouldn’t be used for specific Species
like:
public class Charizard extends Species {
}
but rather for adding additional functionality to all Species
public class ComparableSpecies extends Species implements Comparable {
// ...
}
This happens other times in the code, where an object isn’t logically constructed until all fields are set. If I recall correctly, a constructor should contain all information necessary to logically complete construction of an object, but what happens if that involves a lot of data (like in this example)?
So to the question: Would a Builder
be the right approach for this problem? Is there another pattern that would be better suited for this type of construction? Or have I just gone down the Builder
rabbit hole so far that I can’t see what’s right in my face?
(Note: Don’t worry/focus about/on how changes to the classes will impact any downstream consumers of my API. There are no consumers yet, as all of the code is still private, and not even ready for alpha testing yet)
Any feedback would be appreciated.
1
The Builder pattern is most useful when the product you are building consists of multiple discrete parts, where external information specifies which parts, or if the product has many optional properties.
In your case, you have a long list of required properties. Here the Builder pattern only gives you the appearance of a less complex constructor at the cost of not being able to detect at compile-time that a property was missed.
You are essentially trading code like this
type1 property1 = ...
type2 property2 = ...
type3 property3 = ...
.
.
.
typeN propertyN = ...
Species x = new Species(property1, property2, property3, ..., propertyN);
for
Builder builder;
builder.setProperty1(...);
builder.setProperty3(...);
.
.
.
builder.setPropertyN(...);
Species x = builder.build();
The Species construction looks deliciously simple, but how long will it take you to see why this code doesn’t work?
I know there are guidelines that methods should not take more than 3 (or 4) arguments. However, there is a reason why all languages in practical use don’t impose that as a hard limit: Sometimes you just need more arguments. If even the most aggressive grouping of arguments into parameter objects can’t get you lower than 6 or 7 parameters, then you just need that many.
1
There is no way to logically group these into different objects to reduce the amount of parameters passed into the constructor.
I think this is the essence in your problem.
First, the Builder
pattern is normally used in this situation as best practice. However, for me personally, I don’t like it and even consider it an anti pattern. You in some way too, else you would not ask here, right? 😉
So, I feel like some of the properties can still be grouped. However, a good logical grouping is one of the harder design decisions. So let’s start from your given information (I removed the final for easier reading):
String id,
String name,
Type primaryType,
Type secondaryType,
StatSet baseStats,
StatSet evPointYield
Here it immediatly catches my eye is the type-pairs. While id
and name
looks like they could be grouped, they serve different purposes (identifying vs. displaying). So no grouping here, you are right.
The Type
s and StatSet
s however serve the same purpose – they and only they influence how the species behaves in game interactions. I would therefore group all these into a general Behaviour
object. Its properties can be further grouped. The Type
s could be grouped into a TypeStats
object. The StatSet
s seem to be only used in combat, so that would be the combatStats
. Names are not best chosen here, I admit.
This has some advantages. You can now share common behaviour between different species if you want. You can make sure that when creating TypeStats
that there are none where both first and second type are the same. This should be a concern of the TypeStats
not of the Species
in my opinion. It is also easier to explain the class:
Species contain an Id by which they are identified unique. When playing, they are displayed to the user by their names, depending on the country. Each Species also has their own behaviour in this game. This behaviour consists of how it behaves in combats and what you can do with it when not in combat.
However, my answer may be incomplete or even partly incorrect, depending on the meaning of the properties you mentioned and those that you didnt mention.
One last point: don’t make your second type null
. This is really not good. If you use Java 8, go for the Optional
class. If you use Java below 7, take a look at googles guava
library which provides such a class for you.
4
In times like this I try to step back, forget the implementation, and focus on the requirements.
Essentially, you want to:
– Define discrete entities using primitive data elements
– Validate entities using domain-specific rules
– Construct a domain model from valid entities
– Perform useful and interesting computations with your domain model
If you’re in pre-alpha, the Builder pattern will work just fine to get you started. You could probably skip Builder altogether and just use an interface to hide your setters. Either way, get your most valuable features finished before worrying about low-level implementation details. Your features will drive the requirements of your domain model.
When you have your domain model requirements figured out, you’ll be in a better position to decide if Builder is an acceptable (not necessarily the best) solution. You may find that fields you thought were required aren’t really required at all. For example, does a Pokemon need to own its EV point yield? Could you instead map a Pokemon to its EV point yield in a separate model? With less required fields, Builder may look like a better solution. Useful concepts here include the Law of Demeter and the interface segregation principle.
If the Builder pattern still doesn’t meet your requirements, try looking at data binding with the Java Architecture for XML Binding (JAXB). It’s a standard that defines how to create an entity using data stored as XML, and includes validation as well. Oracle has a basic tutorial to get you started.
If your XML looks like this:
<pokemon>
<id>Charizard</id>
<!-- Other elements -->
</pokemon>
Your Pokemon might look like this:
@XmlRootElement(name="pokemon")
public class Pokemon {
@XmlElement(required=true)
private String id;
}
Normally I try to avoid XML, and JAXB is definitely overkill for small projects. But for large, human-readable data sets with well-defined validation rules that integrate with Java, it works pretty well.
1
One option is to use a parameter object.
Take the parameters required by every instance and bundle them as a seperate class.
See also this question