HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Ready? Set. Fight!

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fightreadyset

Problem

Description

This code is for 'fighting' two objects against each other. I am normally using it to make my AIs fight each other in one game or another.

In case you are wondering: Yes, I am using this in my Tic Tac Toe Ultimate game to determine the skills of the AIs, my TTT Ultimate implementation for the current coding-challenge will be put up here for review later on). This particular Fighting code is however completely independent and is used in many of my other current projects.

Class Summary (489 lines in 7 files, making a total of 14319 bytes)

  • FightInterface.java: Interface to be implemented to determine the winner of a fight



  • FightResults.java: Class to store the results from a bunch of fights



  • GameFight.java: Class that provides the core functionality to create fights and get results



  • GuavaExt.java: Some utility methods, which makes use of Guava, hence the name.



  • PlayerResults.java: Results for a specific player



  • IndexResults.java: Results for when a player plays at a specific index (as there can in some games be differences if you're the first player to play or the second)



  • WinsLosses.java: Stores wins, losses, and draws.



Code

This code can also be found on GitHub

FightInterface.java: (20 lines, 550 bytes)

/**
 * Interface to provide implementation for determining the winner of a fight
 */
public interface FightInterface {
    /**
     * The fight number for the first fight
     */
    int FIRST_FIGHT = 1;

    /**
     * Make two fighters fight each other
     * 
     * @param players The players that should fight each other
     * @param fightNumber Which fight number in the series this is, starting at 1.
     * @return The winner of the fight, or null if it is a draw
     */
    PL determineWinner(PL[] players, int fightNumber);
}


FightResults.java: (126 lines, 3995 bytes)

```
public class FightResults {
private final Map> playerData = new HashMap>();
private final boolean separateIndexes;
private final String

Solution

A couple things I would say:

  • int FIRST_FIGHT = 1 is



  • declared improperly; it should be public static final (it will act this way without the explicit declaration, but I think it makes it clearer to add those modifiers; less to think about)



  • never used



  • not zero-based (any reason?)



  • a little overkill on the constants front, especially if you switch to zero-based, where numeric primitive types are by default initialized to zero



  • Type parameter PL should be P unless you have a good reason otherwise. (T might be even better, but that's up to you.)



  • You shouldn't use PL[] because it's not type-safe. Use List or Collection instead (both from java.util package).



-
Your test case isn't really comprehensive. I've found the most robust tests to be of the following format:

final int TEST_COUNT = 100_000; // or higher
final Random r = new Random();
// set up some counters here
for (int i=0; i < TEST_COUNT; i++) {
    int result = r.nextInt(3);
    switch (result) {
    case 0: // do "P1 win" logic and break;
    case 1: // do "draw" logic and break;
    case 2: // do "P1 loses" logic and break;
    }
    boolean conditionsExpected = /* ... */ ;
    assertTrue(conditionsExpected);
}


This could be in addition to your current test, but it's always good to test things at high loads. This also handles edge conditions and stuff like that. (Make sure your TEST_COUNT is always a good deal higher than the highest argument to r.nextInt.)

  • Your test case shouldn't really print out anything. If you're testing the logic of toStringMultiLine, do some string parsing. If it's just there for debugging, probably remove it.



  • Your exceptions don't generally say much about why the fighters "can't be equal" at the moment.



-
If you're in Java 7 or higher, use that diamond operator <> to turn this

private final Map> playerData = new HashMap>();


into this:

private final Map> playerData = new HashMap<>();

Code Snippets

final int TEST_COUNT = 100_000; // or higher
final Random r = new Random();
// set up some counters here
for (int i=0; i < TEST_COUNT; i++) {
    int result = r.nextInt(3);
    switch (result) {
    case 0: // do "P1 win" logic and break;
    case 1: // do "draw" logic and break;
    case 2: // do "P1 loses" logic and break;
    }
    boolean conditionsExpected = /* ... */ ;
    assertTrue(conditionsExpected);
}
private final Map<PL, PlayerResults<PL>> playerData = new HashMap<PL, PlayerResults<PL>>();
private final Map<PL, PlayerResults<PL>> playerData = new HashMap<>();

Context

StackExchange Code Review Q#41698, answer score: 9

Revisions (0)

No revisions yet.