2

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 the name (usually just the name lowercase). Example: charizard
  • name: the localized name of the Species. Example: Charizard in English, Rizardon in Japanese, and Dracaufeu in French.
  • primaryType: the primary element of this Species. Example: Charizard's primary type is Fire because it is a fire-breathing dragon.
  • secondaryType: (optional) each Species may also have a secondary type. Example: Charizard's secondary type is Flying, but it's pre-evolution does not have wings, so it does not have a secondaryType. null is okay here.
  • baseStats: the base combat abilities of all members of a Species
  • evPointYield: the type of experience gained after defeating a member of a Species

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
  • did you consider wrapping these in Future? Like, defining class fields as private Future<String> id instead of private String id etc
    – gnat
    Commented Jan 15, 2015 at 8:23

4 Answers 4

6

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
  • This. Bite the bullet and use the longer constructor. It will save you untold headache in the long run.
    – Michael
    Commented Jan 15, 2015 at 16:32
3

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 Types and StatSets 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 Types could be grouped into a TypeStats object. The StatSets 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
  • 1
    I thought about this as well, and if it were just these few objects, it would be fine. But I have primaryEggGroup and secondaryEggGroup for breeding (which could be grouped into one EggGroup object), a MoveSet which is a list of all possible moves it can learn by increasing its level, another MoveSet which is a list of all possible moves it can learn with external influence, an ExperienceGroup which determines how much experience is needed to level up, and a primaryAbility, secondaryAbility and hiddenAbility which could be grouped together. Which bumps it up back to 6/7 args.
    – Zymus
    Commented Jan 15, 2015 at 8:58
  • There are also 4-5 additional ones which I haven't listed, simply due to the post limit. Expanded out, there would be 20+ different fields, that would all be required. Reduced, it's up to about 10. Should I just take that as the nature of the beast?
    – Zymus
    Commented Jan 15, 2015 at 9:09
  • Yeah, that nature is what I mean. The problem is, in natural language we are not perfectly abstract in this regards so normally we don't have words for describing these fine-grained abstractions. However, maybe in biology you will find a more appropriate vocabulary for describing these specific property/behaviour groups. The alternative is the builder pattern, but if you go into detail, it leads to the same question: what is the finished state of my current builder called and what is the not yet finished state called? If you know that, you can also just model it explicit with classes.
    – valenterry
    Commented Jan 15, 2015 at 9:29
  • Also, you should regard to seperate the MoveSet from the Species. Better make a Rules like class, that controls, what moves a Species object can learn, depending of the data/state or id of this object. A Species should have moves, but should not know what moves it can learn.
    – valenterry
    Commented Jan 15, 2015 at 9:31
3

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
  • Please don't call XML "human-readable". One can certainly read it somehow, but it is destroying the mind in the long term.
    – valenterry
    Commented Jan 15, 2015 at 18:26
0

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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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