Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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<PL> {
    /**
     * 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<PL> {
    private final Map<PL, PlayerResults<PL>> playerData = new HashMap<PL, PlayerResults<PL>>();
    private final boolean   separateIndexes;
    private final String label;
    private final long timeStart;
    private long timeEnd;

    public FightResults(String label, boolean separateIndexes) {
        this.label = label;
        this.separateIndexes = separateIndexes;
        this.timeStart = System.nanoTime();
    }

    /**
     * @return A list of all the player results, with the worst performing player first and the best player last.
     */
    public List<PlayerResults<PL>> getResultsAsc() {
        List<PlayerResults<PL>> list = new ArrayList<PlayerResults<PL>>(playerData.values());
        Collections.sort(list);
        return list;
    }

    /**
     * @return An ordered {@link LinkedHashMap} ranking all fighters with their associated win percentage with the best performing player first
     */
    public LinkedHashMap<PL, Double> getPercentagesDesc() {
        LinkedHashMap<PL, Double> result = new LinkedHashMap<PL, Double>();
        for (Entry<PL, PlayerResults<PL>> ee : entriesSortedByValues(playerData, true)) {
            result.put(ee.getKey(), ee.getValue().calcTotal().getPercentage());
        }
        return result;
    }

    private static class ProduceValue<PL> implements Function<PL, PlayerResults<PL>> {
        @Override
        public PlayerResults<PL> apply(PL arg0) {
            return new PlayerResults<PL>(arg0);
        }
    }

    void finished() {
        this.timeEnd = System.nanoTime();
    }

    private final ProduceValue<PL> prodValue = new ProduceValue<PL>();
    public void saveResult(PL[] fighters, PL winner) {
        final int DEFAULT_INDEX = 0;
        for (int i = 0; i < fighters.length; i++) {
            PL pp1 = fighters[i];
            PlayerResults<PL> result = GuavaExt.mapGetOrPut(playerData, pp1, prodValue);

            for (int j = 0; j < fighters.length; j++) {
                if (i == j)
                    continue;

                result.addResult(separateIndexes ? i : DEFAULT_INDEX , fighters[j], winner);
            }
        }
    }

    @Override
    public String toString() {
        return playerData.toString();
    }

    /**
     * @return A human-readable text about the results of this fight
     */
    public String toStringMultiLine() {
        StringBuilder str = new StringBuilder();
        if (this.label != null) {
            str.append(this.label);
            str.append("\n");
        }
        str.append(super.toString());
        str.append("\n");
        for (Entry<PL, PlayerResults<PL>> ee : entriesSortedByValues(playerData, false)) {
            str.append(ee.getKey());
            str.append("\n");
            str.append(ee.getValue().toStringMultiLine());
            str.append("\n");
        }
        double timeSpent = (timeEnd - timeStart) / 1000000.0;
        str.append("Time spent: " + (timeSpent) + " milliseconds\n");
        return str.toString();
    }

    /**
     * Sort a map by it's values
     * 
     * @param map The map to sort
     * @param descending True to sort in descending order, false to sort in ascending order
     * @return A SortedSet containing all the entries from the original map.
     */
    public static <K, V extends Comparable<? super V>> SortedSet<Map.Entry<K, V>> entriesSortedByValues(Map<K, V> map, final boolean descending) {
        SortedSet<Map.Entry<K, V>> sortedEntries = new TreeSet<Map.Entry<K, V>>(
            new Comparator<Map.Entry<K, V>>() {
                @Override
                public int compare(Map.Entry<K, V> e1, Map.Entry<K, V> e2) {
                    int res;
                    if (descending) 
                         res = e1.getValue().compareTo(e2.getValue());
                    else res = e2.getValue().compareTo(e1.getValue());
                    return res != 0 ? -res : 1; // Special fix to preserve items with equal values
                }
            }
        );
        sortedEntries.addAll(map.entrySet());
        return sortedEntries;
    }
}

GameFight.java: (85 lines, 3267 bytes)

public class GameFight<PL> {

    private final boolean   separateIndexes;
    private String  label;

    /**
     * @param label The name of this fight, which will be showed in the output
     * @param useSeparateIndexes True to alternate fighters between index 0 and index 1.
     */
    public GameFight(String label, boolean useSeparateIndexes) {
        this.separateIndexes = useSeparateIndexes;
        this.label = label;
    }
    public GameFight(String label) {
        this(label, false);
    }
    public GameFight() {
        this(null);
    }

    /**
     * Creates 1 vs. 1 games for each possible combination of fighters and fights them against each other.
     * 
     * @param fighters An array of the fighters
     * @param gamesPerGroup How many fights each 1 vs. 1 pair should make.
     * @param fightStrategy An interface providing implementation to determine the winner of a fight
     * @return The results of the fighting
     */
    public FightResults<PL> fightEvenly(PL[] fighters, int gamesPerGroup, FightInterface<PL> fightStrategy) {
        FightResults<PL> results = new FightResults<PL>(label, separateIndexes);

        List<List<PL>> groups = GuavaExt.processSubsets(Arrays.asList(fighters), 2);
        for (List<PL> group : groups) {
            PL[] currentFighters = Arrays.copyOf(fighters, 2);
            currentFighters[0] = group.get(0);
            currentFighters[1] = group.get(1);
            if (currentFighters[0] == currentFighters[1])
                throw new IllegalStateException("Fighters cannot be equal at the moment");

            // Fight the games
            for (int i = 1; i <= gamesPerGroup; i++)
                results.saveResult(currentFighters, fightStrategy.determineWinner(currentFighters, i));

            if (separateIndexes) {
                currentFighters[0] = group.get(1);
                currentFighters[1] = group.get(0);
                for (int i = 1; i <= gamesPerGroup; i++)
                    results.saveResult(currentFighters, fightStrategy.determineWinner(currentFighters, i));
            }
        }
        results.finished();
        return results;
    }
    /**
     * Perform a specific number of randomly selected 1 vs. 1 fights.
     * 
     * @param fighters Array of fighters that can be chosen to participate in fights
     * @param count The number of fights to make in total
     * @param fightStrategy An interface providing implementation to determine the winner of a fight
     * @return The results of the fighting
     */
    public FightResults<PL> fightRandom(PL[] fighters, int count, FightInterface<PL> fightStrategy) {
        FightResults<PL> results = new FightResults<PL>(label, separateIndexes);
        Random random = new Random();
        for (int i = 1; i <= count; i++) {
            PL[] currentFighters = Arrays.copyOf(fighters, 2);
            List<PL> playerOptions = new ArrayList<PL>(Arrays.asList(fighters));
            currentFighters[0] = playerOptions.remove(random.nextInt(playerOptions.size()));
            currentFighters[1] = playerOptions.remove(random.nextInt(playerOptions.size()));
            if (currentFighters[0] == currentFighters[1])
                throw new IllegalStateException("Fighters cannot be equal at the moment");

            results.saveResult(currentFighters, fightStrategy.determineWinner(currentFighters, i));
        }
        results.finished();
        return results;
    }
}

GuavaExt.java: (70 lines, 2160 bytes)

/**
 * Utility methods that make a lot of usage of Guava.
 */
public class GuavaExt {
    public static <K, V> V mapGetOrPut(Map<K, V> map, K key, Function<K, V> valueProducer) {
        if (map.containsKey(key))
            return map.get(key);

        V value = valueProducer.apply(key);
        map.put(key, value);
        return value;
    }

    /**
     * Creates a list of all subsets containing the specified number of elements
     * 
     * @param set The items to create combinations of
     * @param subsetSize The size of each of the lists in the result
     * @return List of lists containing all the combinations of lists with the specified size
     */
    public static <T> List<List<T>> processSubsets(List<T> set, int subsetSize) {
        if (subsetSize > set.size()) {
            subsetSize = set.size();
        }
        List<List<T>> result = Lists.newArrayList();
        List<T> subset = Lists.newArrayListWithCapacity(subsetSize);
        for (int i = 0; i < subsetSize; i++) {
            subset.add(null);
        }
        return processLargerSubsets(result, set, subset, 0, 0);
    }

    private static <T> List<List<T>> processLargerSubsets(List<List<T>> result, List<T> set, List<T> subset, int subsetSize, int nextIndex) {
        if (subsetSize == subset.size()) {
            result.add(ImmutableList.copyOf(subset));
        } else {
            for (int j = nextIndex; j < set.size(); j++) {
                subset.set(subsetSize, set.get(j));
                processLargerSubsets(result, set, subset, subsetSize + 1, j + 1);
            }
        }
        return result;
    }

    public static <T> Collection<List<T>> permutations(List<T> list, int size) {
        Collection<List<T>> all = Lists.newArrayList();
        if (list.size() < size) {
            size = list.size();
        }
        if (list.size() == size) {
            all.addAll(Collections2.permutations(list));
        } else {
            for (List<T> p : processSubsets(list, size)) {
                all.addAll(Collections2.permutations(p));
            }
        }
        return all;
    }
}

IndexResults.java: (47 lines, 1071 bytes)

/**
 * The results for when a fighter is at a specific index
 */
public class IndexResults<PL> {
    private final Map<PL, WinsLosses> results = new HashMap<PL, WinsLosses>();

    void informAbout(PL opponent, Boolean winner) {
        WinsLosses winLoss = results.get(opponent);
        if (winLoss == null) {
            winLoss = new WinsLosses();
            results.put(opponent, winLoss);
        }
        if (winner == null) {
            winLoss.draw();
        }
        else if (winner)
            winLoss.win();
        else winLoss.loss();
    }

    public String toStringMultiLine() {
        StringBuilder str = new StringBuilder();
        for (Entry<PL, WinsLosses> ee : results.entrySet()) {
            str.append("vs. ");
            str.append(ee.getKey());
            str.append(": ");
            str.append(ee.getValue());
            str.append("\n");
        }
        return str.toString();
    }

    @Override
    public String toString() {
        return results.toString();
    }

    public WinsLosses calcTotal() {
        return new WinsLosses(results.values());
    }
}

PlayerResults.java: (76 lines, 2082 bytes)

public class PlayerResults<PL> implements Comparable<PlayerResults<PL>> {
    private final PL player;

    public PL getPlayer() {
        return player;
    }
    PlayerResults(PL player) {
        this.player = player;
    }

    private final Map<Integer, IndexResults<PL>> results = new HashMap<Integer, IndexResults<PL>>();

    private final Function<Integer, IndexResults<PL>> producer = new Function<Integer, IndexResults<PL>>() {
        @Override
        public IndexResults<PL> apply(Integer arg0) {
            return new IndexResults<PL>();
        }
    };

    void addResult(int myIndex, PL opponent, PL winner) {
        IndexResults<PL> result = GuavaExt.mapGetOrPut(results, myIndex, producer);
        Boolean winStatus = null;
        if (winner == this.player) // allow for drawed games as winner = null.
            winStatus = true;
        else if (winner != null)
            winStatus = false;
        result.informAbout(opponent, winStatus);
    }

    public String toStringMultiLine() {
        StringBuilder str = new StringBuilder();
        for (Entry<Integer, IndexResults<PL>> ee : results.entrySet()) {
            str.append("as index ");
            str.append(ee.getKey());
            str.append(": (");
            str.append(ee.getValue().calcTotal());
            str.append(")\n");
            str.append(ee.getValue().toStringMultiLine());
        }
        return str.toString();
    }
    @Override
    public String toString() {
        return results.toString();
    }

    public WinsLosses calcTotal() {
        int wins = 0;
        int losses = 0;
        int draws = 0;
        for (Entry<Integer, IndexResults<PL>> ee : results.entrySet()) {
            WinsLosses tot = ee.getValue().calcTotal();
            wins += tot.getWins();
            draws = tot.getDraws();
            losses += tot.getLosses();
        }
        return new WinsLosses(wins, losses, draws);
    }

    public double calculatePercentage() {
        return calcTotal().getPercentage();
    }

    @Override
    public int compareTo(PlayerResults<PL> o) {
        return Double.compare(this.calculatePercentage(), o.calculatePercentage());
    }
}

WinsLosses.java: (65 lines, 1194 bytes)

/**
 * Class to keep track of wins, losses, and draws.
 */
public class WinsLosses {
    private int wins;
    private int losses;
    private int draws;

    public WinsLosses() {
        this(0, 0, 0);
    }
    public WinsLosses(int wins, int losses, int draws) {
        this.wins = wins;
        this.losses = losses;
        this.draws = draws;
    }
    public WinsLosses(Collection<WinsLosses> total) {
        this(0, 0, 0);
        for (WinsLosses winlose : total) {
            wins += winlose.wins;
            losses += winlose.losses;
            draws += winlose.draws;
        }
    }

    void win() {
        wins++;
    }

    void loss() {
        losses++;
    }

    public int getTotal() {
        return wins + draws + losses;
    }

    public int getLosses() {
        return losses;
    }
    public int getWins() {
        return wins;
    }
    public double getPercentage() {
        double drawsBonus = draws / 2.0;
        return (wins + drawsBonus) / (double) getTotal();
    }

    @Override
    public String toString() {
        return String.format("%d wins, %d draws, %d losses (%.2f %%)", wins, draws, losses, getPercentage() * 100);
    }
    void draw() {
        draws++;
    }

    public int getDraws() {
        return draws;
    }

}

Dependencies

  • com.google.common.*: Google's Guava Library

Usage / Test

This code creates simple fighters that each randomize a value from 0 to n and each fighter can have a specified bonus added to it's "score". In each fight, the fighter with the highest score wins.

FightingTest.java: (70 lines, 2289 bytes)

public class FightingTest {
    public static class Fighter {
        private final int random;
        private final int bonus;

        public Fighter(int fightRandom) {
            this(fightRandom, 0);
        }
        public Fighter(int fightRandom, int fightBonus) {
            this.random = fightRandom;
            this.bonus = fightBonus;
        }
        public int fightValue(Random rand) {
            return rand.nextInt(this.random) + this.bonus;
        }
        @Override
        public String toString() {
            return "(Fighter: " + random + "+" + bonus + ")";
        }
    }

    @Test
    public void test() {
        final Random random = new Random(42); // since this is a test we want the results to be stable

        GameFight<Fighter> fight = new GameFight<Fighter>();
        Fighter[] fighters = new Fighter[]{ 
            new Fighter(1), new Fighter(2), new Fighter(3, 2), new Fighter(4), 
            new Fighter(5), new Fighter(6), new Fighter(7), new Fighter(8), new Fighter(9),  
        };
        FightResults<Fighter> results = fight.fightEvenly(fighters, 10000, new FightInterface<Fighter>() {
            @Override
            public Fighter determineWinner(Fighter[] players, int fightNumber) {
                int a = players[0].fightValue(random);
                int b = players[1].fightValue(random);

                return a > b ? players[0] : players[1];
            }
        });
        System.out.println(results.toStringMultiLine());
        System.out.println(results.getPercentagesDesc());

        List<PlayerResults<Fighter>> sortedResults = results.getResultsAsc();
        assertEquals(1, sortedResults.get(0).getPlayer().random);
        assertEquals(2, sortedResults.get(1).getPlayer().random);
        assertEquals(4, sortedResults.get(2).getPlayer().random);
        assertEquals(5, sortedResults.get(3).getPlayer().random);
        assertEquals(6, sortedResults.get(4).getPlayer().random);
        assertEquals(3, sortedResults.get(5).getPlayer().random);
        assertEquals(2, sortedResults.get(5).getPlayer().bonus);
        assertEquals(7, sortedResults.get(6).getPlayer().random);
        assertEquals(8, sortedResults.get(7).getPlayer().random);
    }
}

Output produced by test:

net.zomis.fight.FightResults@f84b0a
(Fighter: 1+0)
as index 0: (0 wins, 0 draws, 80000 losses (0,00 %))
vs. (Fighter: 9+0): 0 wins, 0 draws, 10000 losses (0,00 %)
vs. (Fighter: 3+2): 0 wins, 0 draws, 10000 losses (0,00 %)
vs. (Fighter: 4+0): 0 wins, 0 draws, 10000 losses (0,00 %)
vs. (Fighter: 5+0): 0 wins, 0 draws, 10000 losses (0,00 %)
vs. (Fighter: 7+0): 0 wins, 0 draws, 10000 losses (0,00 %)
vs. (Fighter: 6+0): 0 wins, 0 draws, 10000 losses (0,00 %)
vs. (Fighter: 8+0): 0 wins, 0 draws, 10000 losses (0,00 %)
vs. (Fighter: 2+0): 0 wins, 0 draws, 10000 losses (0,00 %)

(Fighter: 2+0)
as index 0: (14995 wins, 0 draws, 65005 losses (18,74 %))
vs. (Fighter: 9+0): 555 wins, 0 draws, 9445 losses (5,55 %)
vs. (Fighter: 3+2): 0 wins, 0 draws, 10000 losses (0,00 %)
vs. (Fighter: 4+0): 1257 wins, 0 draws, 8743 losses (12,57 %)
vs. (Fighter: 5+0): 1014 wins, 0 draws, 8986 losses (10,14 %)
vs. (Fighter: 1+0): 10000 wins, 0 draws, 0 losses (100,00 %)
vs. (Fighter: 7+0): 727 wins, 0 draws, 9273 losses (7,27 %)
vs. (Fighter: 6+0): 793 wins, 0 draws, 9207 losses (7,93 %)
vs. (Fighter: 8+0): 649 wins, 0 draws, 9351 losses (6,49 %)

(Fighter: 4+0)
as index 0: (32297 wins, 0 draws, 47703 losses (40,37 %))
vs. (Fighter: 9+0): 1596 wins, 0 draws, 8404 losses (15,96 %)
vs. (Fighter: 3+2): 2549 wins, 0 draws, 7451 losses (25,49 %)
vs. (Fighter: 5+0): 2952 wins, 0 draws, 7048 losses (29,52 %)
vs. (Fighter: 1+0): 10000 wins, 0 draws, 0 losses (100,00 %)
vs. (Fighter: 7+0): 2161 wins, 0 draws, 7839 losses (21,61 %)
vs. (Fighter: 6+0): 2453 wins, 0 draws, 7547 losses (24,53 %)
vs. (Fighter: 8+0): 1843 wins, 0 draws, 8157 losses (18,43 %)
vs. (Fighter: 2+0): 8743 wins, 0 draws, 1257 losses (87,43 %)

(Fighter: 5+0)
as index 0: (40963 wins, 0 draws, 39037 losses (51,20 %))
vs. (Fighter: 9+0): 2216 wins, 0 draws, 7784 losses (22,16 %)
vs. (Fighter: 3+2): 4075 wins, 0 draws, 5925 losses (40,75 %)
vs. (Fighter: 4+0): 7048 wins, 0 draws, 2952 losses (70,48 %)
vs. (Fighter: 1+0): 10000 wins, 0 draws, 0 losses (100,00 %)
vs. (Fighter: 7+0): 2822 wins, 0 draws, 7178 losses (28,22 %)
vs. (Fighter: 6+0): 3251 wins, 0 draws, 6749 losses (32,51 %)
vs. (Fighter: 8+0): 2565 wins, 0 draws, 7435 losses (25,65 %)
vs. (Fighter: 2+0): 8986 wins, 0 draws, 1014 losses (89,86 %)

(Fighter: 6+0)
as index 0: (47861 wins, 0 draws, 32139 losses (59,83 %))
vs. (Fighter: 9+0): 2731 wins, 0 draws, 7269 losses (27,31 %)
vs. (Fighter: 3+2): 5005 wins, 0 draws, 4995 losses (50,05 %)
vs. (Fighter: 4+0): 7547 wins, 0 draws, 2453 losses (75,47 %)
vs. (Fighter: 5+0): 6749 wins, 0 draws, 3251 losses (67,49 %)
vs. (Fighter: 1+0): 10000 wins, 0 draws, 0 losses (100,00 %)
vs. (Fighter: 7+0): 3546 wins, 0 draws, 6454 losses (35,46 %)
vs. (Fighter: 8+0): 3076 wins, 0 draws, 6924 losses (30,76 %)
vs. (Fighter: 2+0): 9207 wins, 0 draws, 793 losses (92,07 %)

(Fighter: 3+2)
as index 0: (49760 wins, 0 draws, 30240 losses (62,20 %))
vs. (Fighter: 9+0): 3315 wins, 0 draws, 6685 losses (33,15 %)
vs. (Fighter: 4+0): 7451 wins, 0 draws, 2549 losses (74,51 %)
vs. (Fighter: 5+0): 5925 wins, 0 draws, 4075 losses (59,25 %)
vs. (Fighter: 1+0): 10000 wins, 0 draws, 0 losses (100,00 %)
vs. (Fighter: 7+0): 4306 wins, 0 draws, 5694 losses (43,06 %)
vs. (Fighter: 6+0): 4995 wins, 0 draws, 5005 losses (49,95 %)
vs. (Fighter: 8+0): 3768 wins, 0 draws, 6232 losses (37,68 %)
vs. (Fighter: 2+0): 10000 wins, 0 draws, 0 losses (100,00 %)

(Fighter: 7+0)
as index 0: (53585 wins, 0 draws, 26415 losses (66,98 %))
vs. (Fighter: 9+0): 3428 wins, 0 draws, 6572 losses (34,28 %)
vs. (Fighter: 3+2): 5694 wins, 0 draws, 4306 losses (56,94 %)
vs. (Fighter: 4+0): 7839 wins, 0 draws, 2161 losses (78,39 %)
vs. (Fighter: 5+0): 7178 wins, 0 draws, 2822 losses (71,78 %)
vs. (Fighter: 1+0): 10000 wins, 0 draws, 0 losses (100,00 %)
vs. (Fighter: 6+0): 6454 wins, 0 draws, 3546 losses (64,54 %)
vs. (Fighter: 8+0): 3719 wins, 0 draws, 6281 losses (37,19 %)
vs. (Fighter: 2+0): 9273 wins, 0 draws, 727 losses (92,73 %)

(Fighter: 8+0)
as index 0: (58305 wins, 0 draws, 21695 losses (72,88 %))
vs. (Fighter: 9+0): 3925 wins, 0 draws, 6075 losses (39,25 %)
vs. (Fighter: 3+2): 6232 wins, 0 draws, 3768 losses (62,32 %)
vs. (Fighter: 4+0): 8157 wins, 0 draws, 1843 losses (81,57 %)
vs. (Fighter: 5+0): 7435 wins, 0 draws, 2565 losses (74,35 %)
vs. (Fighter: 1+0): 10000 wins, 0 draws, 0 losses (100,00 %)
vs. (Fighter: 7+0): 6281 wins, 0 draws, 3719 losses (62,81 %)
vs. (Fighter: 6+0): 6924 wins, 0 draws, 3076 losses (69,24 %)
vs. (Fighter: 2+0): 9351 wins, 0 draws, 649 losses (93,51 %)

(Fighter: 9+0)
as index 0: (62234 wins, 0 draws, 17766 losses (77,79 %))
vs. (Fighter: 3+2): 6685 wins, 0 draws, 3315 losses (66,85 %)
vs. (Fighter: 4+0): 8404 wins, 0 draws, 1596 losses (84,04 %)
vs. (Fighter: 5+0): 7784 wins, 0 draws, 2216 losses (77,84 %)
vs. (Fighter: 1+0): 10000 wins, 0 draws, 0 losses (100,00 %)
vs. (Fighter: 7+0): 6572 wins, 0 draws, 3428 losses (65,72 %)
vs. (Fighter: 6+0): 7269 wins, 0 draws, 2731 losses (72,69 %)
vs. (Fighter: 8+0): 6075 wins, 0 draws, 3925 losses (60,75 %)
vs. (Fighter: 2+0): 9445 wins, 0 draws, 555 losses (94,45 %)

Time spent: 164.853973 milliseconds

{(Fighter: 9+0)=0.777925, (Fighter: 8+0)=0.7288125, (Fighter: 7+0)=0.6698125, (Fighter: 3+2)=0.622, (Fighter: 6+0)=0.5982625, (Fighter: 5+0)=0.5120375, (Fighter: 4+0)=0.4037125, (Fighter: 2+0)=0.1874375, (Fighter: 1+0)=0.0}

Questions

  • Is the public interface for these classes clear? Do they seem easy to use?
  • Does these classes seem useful to you?
  • Anything that can be improved, at all, anywhere, somehow? (In one way, the more the better)
share|improve this question
add comment

1 Answer

up vote 6 down vote accepted

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<PL> or Collection<PL> 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<PL, PlayerResults<PL>> playerData = new HashMap<PL, PlayerResults<PL>>();
    

    into this:

    private final Map<PL, PlayerResults<PL>> playerData = new HashMap<>();
    
share|improve this answer
    
When declaring "fields" in an interface, they automatically become public static final. I found it more reasonable to be 1-based as it's not an array, it's just meant as a human understandable counter. It is used in another project that uses this code. PL is used as a short form for "Player", but T could work also. In what way is it not type-safe to use PL[]? Since PL is a generic type, it becomes the more specific type when using specific types. Unfortunately I am using Java 6 for Android compatibility. Your other suggestions seem good. –  Simon André Forsberg Feb 15 at 11:13
    
I'm not sure in what way my testing code isn't 'comprehensible' though? Are you suggesting that I remove the FightInterface implementation from my test and extract it to the testmethod itself? That would make the test useless as the test is meant to show how my Fighting classes are used. I'm not really sure about what you mean here... –  Simon André Forsberg Feb 15 at 14:08
    
Fields: yes, but I think it makes it more readable; your choice. PL: I understand, but (strong) convention is one-character type parameter names, usually starting with T for type, but P would be okay as well. Type-safety: very important, please see this and this. Java6: okay. –  WChargin Feb 15 at 20:58
    
I meant "comprehensive," not "comprehensible." The code is completely readable, but you're only testing a couple values, so it would be easy for something to slip through the cracks. The point of testing is to catch edge cases and stuff; using a ton of random trials simulates real-world use. See here for an example of testing against a built-in implementation with 1000 trials and three possible operations. –  WChargin Feb 15 at 21:01
    
Why does two words sound so similar but mean something entirely different? Well, now I've learned more English at least. The naming convention makes sense, will fix that. Consider the testing more an example of usage instead of testing every single piece of the code. Increasing the number of trials further I can agree with, but what more assertions should be added for these classes? Also, any comments to the usefulness of the code? –  Simon André Forsberg Feb 15 at 21:32
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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