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

Better way to write nested loop, if condition and breaking it

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

Problem

This is a Java method that takes as input a 2 dimensional array called grid.

It uses the i1 and j1 values from when the corresponding grid[i1][j1] position is true for the first time, and then i2 and j2 from when the corresponding grid[i2][j2] position is true for the second time.

A call is then made to drawLine(i1, j1, i2, j2) to draw line between those two points in the screen.

I wrote the following code to take all stuff I want to plot, but the code works but looks bad, is there any better way to write this. This code works fine, I just want to improve on it.

for(int x1 = 0; x1 < rows; x1++) { 
        for (int y1 = 0; y1 < columns; y1++) { 
            if (grid[x1][y1] == true) { 
                int x2 = x1 + 1; 
                for(int y2 = 0; y2 < 500; y2++) { 
                    if (grid[x2][y2] == true) { 
                        drawLine(x1, y1, x2, y2); 
                        break; 
                    } 
                } 
            } 
        } 
    }

Solution

My implementation:

(Is C#.Net but you'll be able to translate easily).

public class Point()
    {
        public int X {get; set;}
        public int Y {get; set;}

        public Point (int x, int y)
        {
            this.X=x;
            this.Y=y;
        }
    }

    public void calcLine(bool[][] grid)
    {
        int rows = 500;
        int columns = 500;

        Point p1 = new Point(0, 0);
        // Change this outer 'if' to 'while' if should continue searching for all lines to draw
        if (canFindNextPoint(grid, rows, columns, p1))
        {
            Point p2 = new Point(p1.X + 1, 0);
            if (canFindNextPoint(grid, rows, columns, p2))
            {
                drawline(p1.X, p1.Y, p2.Y, p2.Y);
            }
        }
    }

    private bool canFindNextPoint(bool[][] grid, int rows, int columns, Point p)
    {
        bool found = false;
        for (int x = p.X; x < rows; x++)
            for (int y = p.Y; y < columns; y++)
            if (grid[x][y])
            {
                found = true;
                p.X = x;
                p.Y = y;
                break;
            }
        return found;
    }

Code Snippets

public class Point()
    {
        public int X {get; set;}
        public int Y {get; set;}

        public Point (int x, int y)
        {
            this.X=x;
            this.Y=y;
        }
    }

    public void calcLine(bool[][] grid)
    {
        int rows = 500;
        int columns = 500;

        Point p1 = new Point(0, 0);
        // Change this outer 'if' to 'while' if should continue searching for all lines to draw
        if (canFindNextPoint(grid, rows, columns, p1))
        {
            Point p2 = new Point(p1.X + 1, 0);
            if (canFindNextPoint(grid, rows, columns, p2))
            {
                drawline(p1.X, p1.Y, p2.Y, p2.Y);
            }
        }
    }

    private bool canFindNextPoint(bool[][] grid, int rows, int columns, Point p)
    {
        bool found = false;
        for (int x = p.X; x < rows; x++)
            for (int y = p.Y; y < columns; y++)
            if (grid[x][y])
            {
                found = true;
                p.X = x;
                p.Y = y;
                break;
            }
        return found;
    }

Context

StackExchange Code Review Q#17553, answer score: 3

Revisions (0)

No revisions yet.