Parsing log files of HearthStone: The API

I am working on a parser that can parse log entries from a game called HearthStone, the overall idea is that it will read the log file live when the game is running, parses the log file and show interesting and useful data in real time.

For this question I focus on the module that provides an API for the actual log entries, and to be more specific I focus on an entry called CreateGameLogEntry. The API is written in Java 6 in order to be compatible with Android.

Class summary:

• LogObject

• Marker interface to denote that a class can be used as an log object.

• A log object can appear anywhere in a log file.

• LogEntry

• Marker interface to denote that a class can be used as an log entry.

• A log entry always ranges over at least one full line of the log file, and thus is also a log object.

• Objects

• Utility class used to mimic JDK 7+ Objects behaviour.

• CreateGameLogEntry

• Entry for [Power] CREATE_GAME.

• Uses the builder pattern.

• CreateGameLogEntry.GameEntityLogEntry

• Entry for [Power] CREATE_GAME - GameEntity.

• Uses the builder pattern.

• CreateGameLogEntry.PlayerLogEntry

• Entry for [Power] CREATE_GAME - Player

• Uses the builder pattern.

• CreateGameLogEntry.PlayerLogEntry.GameAccountId

• Object for [Power] CREATE_GAME - Player - GameAccountId

• Uses the builder pattern.

/**
 * Marker interface to denote that a class can be used as a log object.
 *
 * A log object can appear anywhere in a log file.
 *
 * @author Frank van Heeswijk
 */
public interface LogObject { }

/**
 * Marker interface to denote that a class can be used as an log entry.
 *
 * A log entry always ranges over at least one full line of the log file, and thus is also a log object.
 *
 * @author Frank van Heeswijk
 */
public interface LogEntry extends LogObject { }

```
/**
* Utility class used to mimic JDK 7+ Objects behaviour.
*
* @author Frank van Heeswijk
*/
final class Objects {
private Objects() {
throw new UnsupportedOperationE

Overuse of null checks and abuse of the Builder pattern

You're overusing Objects.requireNonNull and abusing the Builder pattern at multiple places.

private final GameEntityLogEntry gameEntityLogEntry;
    private final Set playerLogEntries;

    private CreateGameLogEntry(final Builder builder) {
        this.gameEntityLogEntry = Objects.requireNonNull(builder.gameEntityLogEntry, "builder.gameEntityLogEntry");
        this.playerLogEntries = Objects.requireNonNull(builder.playerLogEntries, "builder.playerLogEntries");
    }

playerLogEntries in the Builder is never null.
Since Builder is an inner class, under your full control,
the extra null check is either paranoia,
or you wrote that in auto-pilot mode :-)

When you build a CreateGameLogEntry using CreateGameLogEntry.Builder,
if you forget to set gameEntityLogEntry the code will compile.
That's not great,
because a CreateGameLogEntry without a GameEntityLogEntry doesn't make sense,
and sure enough,
thanks to the null check in the constructor,
the problem will manifest in an NPE at runtime.
Problems showing up at runtime are not great when you can prevent them at compile time.
And you can,
by making GameEntityLogEntry a constructor parameter in the Builder.

If GameEntityLogEntry is never missing,
a user might still willfully set a null value explicitly,
but would that really be a problem?
If not, then you can also remove the Objects.requireNonNull check,
further reducing boilerplate code.

Adding constructor parameters to a Builder might seem like it defeats the purpose.
But it doesn't.
It's a (useful!) variation on the classic Builder pattern, a twist, if you will.

Extra caution about the Builder pattern

Keep in mind that the Builder pattern has some overhead:
it's quite tedious to write up.
It's most useful when you have 4+ optional parameters.
Under 4 parameters, it's often easier to use classic constructors.

With mandatory arguments the pattern becomes an unwieldy mix.
Because, the only way to ensure integrity of the class at compile time is to move those parameters into the Builder's constructor.
This goes against the pattern,
weakening its usefulness.

About defensive getters

In your defensive getters like this one:

public Set getPlayerLogEntries() {
        return new HashSet(playerLogEntries);
    }

An interesting alternative can be to return an unmodifiable set instead of creating a new instance:

public Set getPlayerLogEntries() {
        return Collections.unmodifiableSet(playerLogEntries);
    }

StackExchange Code Review Q#77392, answer score: 3