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.

This question is about my answer to this king-of-the-hill challenge on Code Golf. In short, the purpose of the challenge is to code a slime that, trough various possible moves gains control over an 8x8 play area against 3 other slimes. The slimes get their turn one after another.

My idea was to go trough all possible moves for the next few rounds, put a score on each possible outcome and then estimating which possible move would be the best. I decided I should at least look as far as after my own next move, but as there are somewhere around 100 possible moves each turn, this quickly gets too slow. I now randomly skip a certain percentage of possible moves, based on a guess of how many moves there are:

double skip = (us*2.5/(us*2.5 + 1))/4 + 0.735;

Where us is an integer that indicates how many of the currently active player are on the board. On the first turn, I don't skip any moves and on the second I multiply the skip ratio by itself, since the base of the tree is more important to me. As you can see, I am skipping a lot of branches.

TL; DR: I'm doing some tree searches on the board states of a game, I need to get the performance up.

How can I improve my code to run faster so I don't need to skip so many branches? Seeing as I'm not a very experienced programmer at all and Java is very new to me, I'm sure the performance can be orders of magnitude better.

A few things I can think of that could use improving:

  • I have a fixed size array to store boardstates that have already been over (since some moves have the same outcome) the way I handle this might be slowing me down.
  • Am I returning values that don't need returning since they are passed as a reference?
  • Should I be storing the boardstates more efficiently?
  • My choice of language might not be the best.
  • Though probably not directly related to performance, the main loop in my getBestMove method is a mess.

Any other feedback on my code is of course also welcomed, but performance goes first in this case.

Here's my full code:

import java.awt.Point;  

public class ShallowBlue {
    private static final int MAX_ROUNDS = 5, PLAYERS = 4;
    static int me = 0;

    public static void main(String[] args) {
        if (args[0] == null) {
                return;
        }

    me = Integer.parseInt(args[0].split(",", 2)[0]);
    String board = args[0].split(",", 2)[1];

    System.out.println(getBestMove(board, me, MAX_ROUNDS - 1));
}

private static String getBestMove(String board, int player, int rounds) {
    String [] boards = new String[24];
    int checkedBoards = 1;
    char playerChar = Integer.toString(player).charAt(0);
    String tempMove = getMove(0, 0, 0, 0);
    String tempBoard = calculateMove(board, tempMove); 
    boards[0] = tempBoard;
    String bestMove = tempMove;
    double us = numberOfUs(board, playerChar); 
    double skip = (us*2.5/(us*2.5 + 1))/4 + 0.735;
    if (rounds == MAX_ROUNDS - 2) {
        skip = skip*skip;
    }

    float bestScore, worstScore, averageScore, tempScore;
    int scores;

    if (rounds == 0) {
        tempScore = calculateScore(tempBoard, MAX_ROUNDS - rounds - 1);
    } else {
        tempScore = getScore(getBestMove(tempBoard, player%PLAYERS + 1, rounds - 1));
    }

    scores = 1;
    bestScore = tempScore;
    worstScore = tempScore;
    averageScore = tempScore;

    for (int x = 0; x < 8; x++) {
        for (int y = 0; y < 8; y++) {
            if (getCharAt(board, x, y) == playerChar) {
                Point[] possibleMergers = getNeighboringMatches(board, new Point(x, y), playerChar);
                if (possibleMergers[0] != null) {
                    tempMove = getMove(possibleMergers[0].x, possibleMergers[0].y, x, y); 
                    tempBoard = calculateMove(board, tempMove);
                    if (addIfUnique(boards, tempBoard, checkedBoards)) {
                        checkedBoards++;
                        if ((rounds != MAX_ROUNDS - 1) && (rounds == 0 || Math.random() < skip)) {
                            tempScore = calculateScore(tempBoard, MAX_ROUNDS - rounds - 1);
                        } else {
                            tempScore = getScore(getBestMove(tempBoard, player%PLAYERS + 1, rounds - 1));
                        }

                        if (tempScore > bestScore) {
                            bestMove = tempMove;
                        }
                        bestScore = Math.max(tempScore, bestScore);
                        worstScore = Math.min(tempScore, worstScore);

                        scores++;
                        averageScore = (averageScore*(scores - 1) + tempScore)/scores;
                    }
                }
            } else if (getCharAt(board, x, y) == '.') {
                Point[] possibleSpreaders = getNeighboringMatches(board, new Point(x, y), playerChar);
                int i = 0;
                while (i < possibleSpreaders.length && possibleSpreaders[i] != null) {
                    tempMove = getMove(possibleSpreaders[i].x, possibleSpreaders[i].y, x, y); 
                    tempBoard = calculateMove(board, tempMove);
                    if ((rounds != MAX_ROUNDS - 1) && (rounds == 0 || Math.random() < skip)) {
                        tempScore = calculateScore(tempBoard, MAX_ROUNDS - rounds - 1);
                    } else {
                        tempScore = getScore(getBestMove(tempBoard, player%PLAYERS + 1, rounds - 1));
                    }

                    if (tempScore > bestScore) {
                        bestMove = tempMove;
                    }
                    bestScore = Math.max(tempScore, bestScore);
                    worstScore = Math.min(tempScore, worstScore);

                    scores++;
                    averageScore = (averageScore*(scores - 1) + tempScore)/scores;

                    i++;
                }
                Point[] possibleJumpers = getNextNeighboringMatches(board, new Point(x, y), playerChar);
                i = 0;
                while (i < possibleJumpers.length && possibleJumpers[i] != null) {
                    tempMove = getMove(possibleJumpers[i].x, possibleJumpers[i].y, x, y); 
                    tempBoard = calculateMove(board, tempMove);
                    if ((rounds != MAX_ROUNDS - 1) && (rounds == 0 || Math.random() < skip)) {
                        tempScore = calculateScore(tempBoard, MAX_ROUNDS - rounds - 1);
                    } else {
                        tempScore = getScore(getBestMove(tempBoard, player%PLAYERS + 1, rounds - 1));
                    }

                    if (tempScore > bestScore) {
                        bestMove = tempMove;
                    }
                    bestScore = Math.max(tempScore, bestScore);
                    worstScore = Math.min(tempScore, worstScore);

                    scores++;
                    averageScore = (averageScore*(scores - 1) + tempScore)/scores;

                    i++;
                }
            }
        }
    }

    if (rounds == MAX_ROUNDS - 1) {
        return (bestMove);
    } else {
        return getScoreString(bestScore, worstScore, averageScore);
    }
}

private static int numberOfUs(String board, char playerChar) {
    int us = 0;

    for (int i = 0; i < board.length(); i++ ) {
         if (board.charAt(i) == playerChar) {
            us++;
        }
    }

    return us;
}

private static float calculateScore(String board, int roundsPassed) {
    int empties = 0;
    int us = 0;
    int enemy1 = 0;
    int enemy2 = 0;
    int enemy3 = 0;
    for (int i = 0; i < board.length(); i++ ) {
        if (board.charAt(i) == '.') {
            empties++;
        } else if (board.charAt(i) == Integer.toString(me).charAt(0)) {
            us++;
        } else if (board.charAt(i) == Integer.toString(me%PLAYERS + 1).charAt(0)) {
            enemy1++;
        } else if (board.charAt(i) == Integer.toString(me%PLAYERS + 2).charAt(0)) {
            enemy2++;
        } else if (board.charAt(i) == Integer.toString(me%PLAYERS + 3).charAt(0)) {
            enemy3++;
        }
    }

    if (us != 0) {
        us += roundsPassed;
    }

    if (enemy1 != 0) { 
        enemy1 = enemy1 + (roundsPassed + 3)%PLAYERS;
    }

    if (enemy2 != 0) { 
        enemy2 = enemy2 + (roundsPassed + 2)%PLAYERS;
    }

    if (enemy3 != 0) { 
        enemy3 = enemy3 + (roundsPassed + 1)%PLAYERS;
    }

    return us*(empties + 1)/(Math.max(Math.max(enemy1, enemy2), enemy3) + 1);
}

private static float getScore(String scoreString) {
    float bestScore, worstScore, averageScore;
    String[] scores = new String[3];

    scores = scoreString.split(",");
    bestScore = Float.parseFloat(scores[0]);
    worstScore = Float.parseFloat(scores[1]);
    averageScore = Float.parseFloat(scores[2]);


    return (float) Math.sqrt(Math.sqrt(bestScore*averageScore*worstScore*worstScore));
}

private static String getScoreString(float bestScore, float worstScore, float averageScore) {
    return Float.toString(bestScore) + ',' + Float.toString(worstScore) + ',' + Float.toString(averageScore);
}

private static boolean addIfUnique(String[] boards, String board, int checkedBoards) {
    int i = 0;

    while (i < boards.length && boards[i] != null) {
        if (boards[i].equals(board)) {
            return false;
        }
        i++;
    }

    if (i < boards.length) {
        boards[i] = board;
    } else {
        boards[checkedBoards%boards.length] = board;
    }

    return true;
}

private static String calculateMove(String board, String move) {
    int x1 = Integer.parseInt(Character.toString(move.charAt(0)));
    int y1 = Integer.parseInt(Character.toString(move.charAt(2)));
    int x2 = Integer.parseInt(Character.toString(move.charAt(4)));
    int y2 = Integer.parseInt(Character.toString(move.charAt(6)));

    if ((Math.abs(y1 - y2) == 2 || Math.abs(x1 - x2) == 2) 
            &&  getCharAt(board, x2, y2) == '.') {
        Point[] enemies = new Point[8];

        enemies = getNeighboringEnemies(board, new Point(x1, y1), Integer.parseInt(Character.toString(getCharAt(board, x1, y1))));

        board = replace(board, enemies, getCharAt(board, x1, y1));
        Point[] middle = {new Point(x1, y1)};
        board = replace(board, middle, '.');
    }

    if ((Math.abs(y1 - y2) == 1 || Math.abs(x1 - x2) == 1)) { 
        if (getCharAt(board, x2, y2) == '.' || getCharAt(board, x1, y1) == getCharAt(board, x2, y2)) {
            boolean merge = true;
            if (getCharAt(board, x2, y2) == '.') {
                merge = false;
            }

            Point[] spaces = new Point[8];
            spaces = getNeighboringMatches(board, new Point(x1, y1), '.');
            board = replace(board, spaces, getCharAt(board, x1, y1));

            if (merge) {
                Point[] source = {new Point(x1, y1)};
                board = replace(board, source, '.');
            }
        }
    }

    return board;
}

private static String replace(String board, Point[] targets, char source) {
    int i = 0;

    while (i < targets.length && targets[i] != null) {
        if (targets[i].x == 7 && targets[i].y == 7) {
            board = board.substring(0, getIndexAt(targets[i].x, targets[i].y)) + source;
        } else if (targets[i].x == 0 && targets[i].y == 0) {
            board = source + board.substring(getIndexAt(targets[i].x, targets[i].y) + 1);
        } else {
            board = board.substring(0, getIndexAt(targets[i].x, targets[i].y)) + source + board.substring(getIndexAt(targets[i].x, targets[i].y) + 1);
        }
        i++;
    }

    return board;
}

private static Point[] getNeighboringMatches(String board, Point coord, char match) {
    Point[] matches = new Point[8];

    int i = 0;
    for (int x = coord.x - 1; x <= coord.x + 1; x++) {
        for (int y = coord.y - 1; y <= coord.y + 1; y++) {
            if ((y != coord.y || x != coord.x ) && getCharAt(board, x, y) == match){
                matches[i] = new Point(x, y);
                i++;
            }
        }
    }

    return matches;
}

private static Point[] getNeighboringEnemies(String board, Point coord, int player) {
    Point[] enemies = new Point[8];

    for (int i = 1; i <= PLAYERS; i++){
        enemies = mergeArr(enemies, getNeighboringMatches(board, coord, Integer.toString((player + i - 1)%PLAYERS + 1).charAt(0)));
    }

    return enemies;
}

private static Point[] getNextNeighboringMatches(String board, Point coord, char match) {
    Point[] matches = new Point[16];

    int i = 0;
    for (int x = coord.x - 2; x <= coord.x + 2; x++) {
        for (int y = coord.y - 2; y <= coord.y + 2; y++) {
            if ((Math.abs(y - coord.y) == 2 || Math.abs(x - coord.x) == 2) && getCharAt(board, x, y) == match){
                matches[i] = new Point(x, y);
                i++;
            }
        }
    }

    return matches;
}

private static char getCharAt(String board, int x, int y) {

    if (x >= 0 && x < 8 && y >= 0 && y < 8) {
        return board.charAt(9*x + y);
    } else {
        return '\0';
    }
}

private static int getIndexAt(int x, int y) {
    return 9*x + y;
}

private static Point[] mergeArr(Point[] arr1, Point[] arr2) {
    int i = 0;
    int j = 0;

    while (i < arr1.length && arr1[i] != null) {
        i++;
    }

    while (j < arr2.length && arr2[j] != null) {
        arr1[i + j] = arr2[j];
        j++;
    }

    return arr1;
}

private static String getMove(int x1, int y1, int x2, int y2) {
    return Integer.toString(x1) + " " + Integer.toString(y1) + " " + Integer.toString(x2) + " " + Integer.toString(y2);
    }
}
share|improve this question
1  
Did you read up on commonly used algorithms like alpha-beta pruning that help to reduce the search space? –  Nobody Jul 23 at 9:07
    
I just read up on it, but it's not particularly helpful since I'm only getting 5 entire round deep, and as there are 4 players and not 2, I could only start eliminating trees at the 5th level. It did give me an idea for a sub-optimal system that will run quicker though. –  overactor Jul 23 at 9:56
    
I forgot that it was only for two players but it should be a good starting point to find similar techniques for a more general solution. –  Nobody Jul 23 at 10:00

1 Answer 1

up vote 6 down vote accepted

Well, this is quite a bit of code so just some generic remarks and advise for now:

  1. Your code is a prime example of what I would call spaghetti code. It's unmaintainable and if you come back to it in 6 months time you'll have a hard time understanding what all of it is doing.

  2. Just from skimming over the code there seems to be a lot of code repetition especially in getBestMove.

  3. Strings in Java are immutable. Which means any operation which modifies a string actually creates a new string. Also you do a lot of parsing and converting back and forth between strings and numbers which won't help with the speed issue.

So my general advise would be: Create a dedicated Board class which encapsulates the board access and operations performed on the board. As backing store I'd probably chose a 2d int-array representing the cells. If you need to print it on screen you can have a convenient output method which converts it into the string representation.

This will get rid of a lot of unnecessary converting to and from strings and should result in some speedup (how much exactly I can't say). It will also provide a starting point for a better code structure.

share|improve this answer
    
Yourpoint about spaghetti code makes a lot of sense. I started this code assuming it would be rather simple and didn't pay much attention to structure. Things got out of hand rather quickly though. –  overactor Jul 23 at 7:26
    
Would an array of bytes produce measurably faster code than an array of int? I've read here that in arrays, bytes get packed. This should result in 4 times less memory usage for boards, right? Then again, calculations with bytes seem to be slower than those with ints. –  overactor Jul 23 at 8:20
    
It might be faster or it might not be. In the end accessing individual bytes requires un-aligned memory access or byte masking and shifting. On the other hand it might benefit from better caching due too smaller size. My guess would be that it won't make a huge difference but you'll have to try to find out –  ChrisWue Jul 23 at 8:31

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.