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

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;
}
}
}
}
}

-

## migrated from stackoverflow.comOct 15 '12 at 13:33

This question came from our site for professional and enthusiast programmers.

Watever language the above code is, Its a small piece of code so performance wont matter much unless you are providing really large data. However, declaring a integer inside two for loops may decrease the performance. – Anil Oct 12 '12 at 0:55
Also, what's the significance of that "magic number" 500. You should declare this as a well-made constant so that its meaning is clear (and so it can be re-used more easily without needing to update its value in more than one place). – Sepster Oct 12 '12 at 0:56
Your break statement breaks out of only the inner-most for loop. Do you intend for it to continue looking for other lines to draw? – phatfingers Oct 13 '12 at 3:47

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;
}

-
Which is actually pretty much a concrete implementation of @Algebra's answer, I think. – Sepster Oct 12 '12 at 1:29
I understood your idea but I cannot write this in Java as I cannot pass parameters by reference in Java? like is there any "passing parameters by value" alternative of this code? – user1739762 Oct 12 '12 at 1:54
Ouch. A cleaner approach would be to wrap your x,y values as a "Point" object, with x and y properties. You can then pass your "point" in (and while the reference to your point object will be passed as a by-value parameter, you'll still be able to work on the point object that is referenced, and its new values will be available to the calling scope. – Sepster Oct 12 '12 at 2:12
@user1739762 Have updated answer to reflect this suggestion. Thanks for the accept! – Sepster Oct 12 '12 at 2:17
Thank you, I really appreciate your effort. – user1739762 Oct 12 '12 at 2:30

I would create a support method that knows to look for the next true value in the grid

void FindTrue(int &x, int &y, bool[][] grid)
{
for(; x < rows; x++, y = 0)
for(; y < columns; y++)
if (grid[x][y]) return;
}


now your method can call this method like this:

1. With x=0,y=0 and after that store them to x1, x2.
2. Advance x, y by one locating (may need to drop a line).
3. With untouched x, y.
4. Call drawLine(x1,y1,x,y)

consider this pseudo code cuz I am not fluent in java.

-
+1. NB my answer is roughly a concrete (albeit C#.Net, not java) implementation of this answer. – Sepster Oct 12 '12 at 1:39

try the while loop

while(true) {
int x1 = 0;
if(x1 < rows) break;

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;
}
}
}
}
x1++;
}

-