3
\$\begingroup\$

I have a Java class which accepts some data and reads it in as a raster of characters x by y. The class is used then to look for occurrences of a certain word in any direction of the raster and store them in a multidimensional array.

My code works, but I would like to see the code optimized because to me it looks too cumbersome with all the if-else's.

...
private static final int NORTH = 0;
private static final int NORTH_EAST = 1;
private static final int EAST = 2;
private static final int SOUTH_EAST = 3;
private static final int SOUTH = 4;
private static final int SOUTH_WEST = 5;
private static final int WEST = 6;
private static final int NORTH_WEST = 7;

/** The raster <code>char[yIndex][xIndex]</code> */
private char[][] raster;

/**
 * Starts looking through the raster to find occurrences of the given word.
 * @param word the word to look for
 */
public void searchWord(String word) {
    if (!searched || !this.word.equals(word)) {
        this.word = word;
        this.count = 0;
        int w = raster[0].length, h = raster.length;
        for (int y=0; y<h; y++) {
            for (int x=0; x<w; x++) {
                this.occurrences = (int[][][]) ArrayUtils.addAll(
                    this.occurrences, 
                    search(x, y, w, h, toLowerCase(this.word.toCharArray())));
                    // toLowerCase is a helper method which converts a char array 
                    // to lower case characters
                this.count = this.occurrences.length;
            }
        }
        searched = true;
    }
}

/**
 * Looks at a given index in the raster for the occurrence of a word to 
 * <code>lookFor</code>.
 * The word is searched horizontally, vertically and diagonally, forward and reverse.
 *  
 * @param x start looking at this x-coordinate
 * @param y start looking at this y-coordinate
 * @param w width of the raster
 * @param h height of the raster
 * @param lookFor characters of the word to look for
 * @return the occurrences found for this word:
 *         int [occurrenceIndex][characterIndex][x,y coordinate] 
 */
private int [][][] search(int x, int y, int w, int h, char [] lookFor) {
    int len = lookFor.length, occurrences[][][] = new int [8][len][];
    for (int j=0; j<8; j++) {
        for (int i=0; i<len; i++) {
            if (i == 0) {
                occurrences[j][0] = new int [] {x, y};
            } else {
                occurrences[j][i] = new int [] {-1, -1};
            }
        }
    }
    boolean north = y - (len-1) >= 0,
            east  = x + len <= w,
            west  = x - (len-1) >= 0,
            south = y + len <= h,
            northEast = north && east,
            northWest = north && west,
            southEast = south && east,
            southWest = south && west;
    for (int i=0; i<lookFor.length; i++) {
        if (north && raster[y-i][x] == lookFor[i]) {
            occurrences[NORTH][i][0] = x;
            occurrences[NORTH][i][1] = y-i;
        } else {
            occurrences[NORTH] = null;
            north = false;
        }
        if (northEast && raster[y-i][x+i] == lookFor[i]) {
            occurrences[NORTH_EAST][i][0] = x+i;
            occurrences[NORTH_EAST][i][1] = y-i;
        } else {
            occurrences[NORTH_EAST] = null;
            northEast = false;
        }
        if (east && raster[y][x+i] == lookFor[i]) {
            occurrences[EAST][i][0] = x+i;
            occurrences[EAST][i][1] = y;
        } else {
            occurrences[EAST] = null;
            east = false;
        }
        if (southEast && raster[y+i][x+i] == lookFor[i]) {
            occurrences[SOUTH_EAST][i][0] = x+i;
            occurrences[SOUTH_EAST][i][1] = y+i;
        } else {
            occurrences[SOUTH_EAST] = null;
            southEast = false;
        }
        if (south && raster[y+i][x] == lookFor[i]) {
            occurrences[SOUTH][i][0] = x;
            occurrences[SOUTH][i][1] = y+i;
        } else {
            occurrences[SOUTH] = null;
            south = false;
        }
        if (southWest && raster[y+i][x-i] == lookFor[i]) {
            occurrences[SOUTH_WEST][i][0] = x-i;
            occurrences[SOUTH_WEST][i][1] = y+i;
        } else {
            occurrences[SOUTH_WEST] = null;
            southWest = false;
        }
        if (west && raster[y][x-i] == lookFor[i]) {
            occurrences[WEST][i][0] = x-i;
            occurrences[WEST][i][1] = y;
        } else {
            occurrences[WEST] = null;
            west = false;
        }
        if (northWest && raster[y-i][x-i] == lookFor[i]) {
            occurrences[NORTH_WEST][i][0] = x-i;
            occurrences[NORTH_WEST][i][1] = y-i;
        } else {
            occurrences[NORTH_WEST] = null;
            northWest = false;
        }
    }
    int [][] compareTo = new int[lookFor.length][];
    for (int i=0; i<lookFor.length; i++) {
        compareTo[i] = new int[] {-1, -1};
    }
    ArrayList<int[][]> items = new ArrayList<int[][]>();
    for (int [][] dirArray : occurrences) {
        if (dirArray != null) {
            items.add(dirArray);
        }
    }
    return (int[][][]) items.toArray(new int [][][] {});
}
...
\$\endgroup\$
2
\$\begingroup\$

You could theoretically make it nicer by introducing Iterator<> concept into the code. It would make your code more optimal from readability point of view but I doubt it would make the code more optimal from execution speed point of view. Your Iterator<> interface derivates would always move to the next character in given direction. Basically, the ifs would be hidden behind the iterators.

If using Iterator<> seems too heavy, you could abstract the position increments and direction into an array of (spatial) vectors. E.g., iteration increment in the north direction could be represented by i * {0, +1}. This way you could transform the if/else chain into iteration over an array of such vectors.

I would also introduce a class for the point/coordinate instead of using two element array.

All of this could improve the abstraction and readability. It is up to you to decide if it is worth it though.

\$\endgroup\$
  • \$\begingroup\$ could you give an example of how I would implement the spatial vectors, I tried this already but it didn't go as expected \$\endgroup\$ – jb10210 Apr 11 '12 at 9:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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