ConstructionCat
ConstructionCat

Reputation: 55

How to create shorter if-statements, or make them look better?

I have this code, where the conditions are very similar and the methods being called are the same. I was wondering if there's a way to make this look better, or at least make it smaller and thus easier to read.

public void open(int i, int j){
    if (i > n - 1 || i < 0 || j > n - 1 || j < 0) {
    throw new IndexOutOfBoundsException
        ("Index out of bounds.");
    }
    grid[i][j] = true;
    
    if (i - 1 >= 0 && grid[i - 1][j]) {
        unionFind.union(location(i - 1, j), location(i, j));
        unionFind2.union(location(i - 1, j), location(i, j));
    }
    
    if (i + 1 < n && grid[i + 1][j]) {
        unionFind.union(location(i + 1, j), location(i, j));
        unionFind2.union(location(i + 1, j), location(i, j));
    }
    
    if (j - 1 >= 0 && grid[i][j - 1]) {
        unionFind.union(location(i, j - 1), location(i, j));
        unionFind2.union(location(i, j - 1), location(i, j));
    }
    
    if (j + 1 < n && grid[i][j + 1]) {
        unionFind.union(location(i, j + 1), location(i, j));
        unionFind2.union(location(i, j + 1), location(i, j));
    }
    numberOpen++;
}

Upvotes: 1

Views: 192

Answers (3)

user4910279
user4910279

Reputation:

Try this.

static final int[][] DIRECTIONS = {{-1, 0}, {1, 0}, {0, -1}, {0, 1}};

boolean inRange(int i, int j) {
    return i >= 0 && i < n && j >= 0 && j < n;
}

public void open(int i, int j) {
    if (!inRange(i, j))
        throw new IndexOutOfBoundsException("Index out of bounds.");
    grid[i][j] = true;
    for (int[] dir : DIRECTIONS) {
        int ii = i + dir[0], jj = j + dir[1];
        if (inRange(ii, jj) && grid[ii][jj]) {
            unionFind.union(location(ii, jj), location(i, j));
            unionFind2.union(location(ii, jj), location(i, j));
        }
    }
    numberOpen++;
}

Upvotes: 0

Ajit Kumar
Ajit Kumar

Reputation: 181

private int[] dr = new int[] {-1, 1, 0, 0};
private int[] dc = new int[] { 0, 0, -1, 1};


public void open(int i, int j) {
    if (i > n - 1 || i < 0 || j > n - 1 || j < 0) {
        throw new IndexOutOfBoundsException("Index out of bounds.");
    }
    grid[i][j] = true;

    Location thisLocation = new location(i, j);

    for (int k = 0; k < 4; i++) {
        int newI = dr[k] + i;
        int newJ = dc[k] + j;
        performUnion(new Location(newI, newJ), thisLocation);
    }
    
    numberOpen++;
}

private void performUnion(Location loc, Location unionWith) {
    if (loc.x < 0 || loc.y < 0 || loc.x >= n || loc.y >= n && !grid[loc.x][loc.y]) {
        return;
    }
    unionFind.union(loc, unionWith);
    unionFind2.union(loc, unionWith);
}

Upvotes: 0

Jon Skeet
Jon Skeet

Reputation: 1499790

Well, you could easily write a method which conditionally calls the union method, then call that unconditionally from open:

public void open(int i, int j) {
    if (i > n - 1 || i < 0 || j > n - 1 || j < 0) {
        throw new IndexOutOfBoundsException("Index out of bounds.");
    }
    grid[i][j] = true;
    // I'm assuming the result of this doesn't change between calls?
    // (We don't know the type, either.)
    Location thisLocation = location(i, j);
    maybeUnion(i - 1, j, thisLocation);
    maybeUnion(i + 1, j, thisLocation);
    maybeUnion(i, j + 1, thisLocation);
    maybeUnion(i, j - 1, thisLocation);
    numberOpen++;
}

private void maybeUnion(int x, int y, Location unionWith) {
    if (x < 0 || y < 0 || x >= n || y >= n || !grid[x][y]) {
        return;
    }
    unionFind.union(location(x, y), unionWith);
    unionFind2.union(location(x, y), unionWith);
}

Upvotes: 2

Related Questions