6
\$\begingroup\$

I want to 180 rotate an Image. Here's what I wrote but it seems way too complicated. Is there any way to do this in a simpler way?

public void rotateImage180Deg() {
        for (int x = 0; x < width / 2; x++) {

            int y = width - 1 - x;
            for (int i = x; i < y; i++) {
                int offset = i - x;
                RGBColor top = imageMatrix[x][i];
                imageMatrix[x][i] = imageMatrix[y][y - offset];
                imageMatrix[y][y - offset] = top;
                RGBColor leftBottom = imageMatrix[y - offset][x];
                imageMatrix[y - offset][x] = imageMatrix[i][y];
                imageMatrix[i][y] = leftBottom;
            }
        }
    }
\$\endgroup\$
  • 4
    \$\begingroup\$ Is it a given that image is always square? \$\endgroup\$ – holroy May 13 '17 at 17:48
  • 1
    \$\begingroup\$ What is the coordinates of your upper left corner? \$\endgroup\$ – holroy May 13 '17 at 18:15
  • \$\begingroup\$ @holry, actually not, isn't this supposed to work for non-square images? \$\endgroup\$ – Baldr May 13 '17 at 18:55
4
\$\begingroup\$

Your code is complicated and hard to read, say the least. So I'll start discussing dimensions of arrays in java, and ease into a re-implementation of your code. Some lessons to be learned is added to the end.

If you create a two-dimensional array in java, int[2][3] twoDim, and use Arrays.deepToString(twoDim) (from java.util.Arrays) to print it. It'll print something ala: [[0, 1, 2], [3, 4, 5]]. And twoDim[1][2] has the value 5. This I interpret that the first dimension is zero-based rows, and the second dimension is zero-based columns.

In your code you intermix x and y and use the width when calculating the y (which I read as height or rows). In addition you add an i and offset to the mix which makes everything very hard to read.

So I plain and simply gave up trying to understanding what your algorithm tries to do. The last two variables further add to the confusion with one top and the other leftBottom. Is the top to the right or left, and what happens when you start iterating?

Helper methods

In order to try figuring out what is really happening I made these three helper methods and variables:

final int height = 3;
final int width = 3;
int[][] imageMatrix = new int[height][width];

public void swap(int x1, int y1, int x2, int y2) {
  int tmp;
  tmp = imageMatrix[y1][x1];
  imageMatrix[y1][x1] = imageMatrix[y2][x2];
  imageMatrix[y2][x2] = tmp;
}

public void resetMatrix() {
  int k = 0;
  for (int row = 0; row < height; row++) {
    for (int col = 0; col < width; col++) {
      imageMatrix[row][col] = k++;
    }
  }
}

public void printMatrix(String title) {
  System.out.println(title);
  for (int row = 0; row < height; row++) {
    for (int col = 0; col < width; col++) {
      System.out.printf("%2d ", imageMatrix[row][col]);
    }
    System.out.println();
  }
  System.out.println();
}

This allowed me to have a slightly simpler structure to work with, and I can display the matrix whenever I feel like it.

On a square image of 3x3 I got the following output:

 Original
  0  1  2 
  3  4  5 
  6  7  8 

 rotateImage180Deg
  8  7  6 
  5  4  3 
  2  1  0 

I then proceeded to changed your algorithm using the swap helper method (and added some space to align swap coordinates), this gave this code:

public void rotateImage180DegII() {
     for (int x = 0; x < width / 2; x++) {

         int y = width - 1 - x;
         for (int i = x; i < y; i++) {
             swap(i, x,            y - i + x, y);
             swap(x, y - i + x,    y, i);
         }
     }
 }   

Output was the same, as expected, and it does fail with the same (but more on that later). In the code above, you clearly see how strange the usage of x and y becomes, as you intermix them freely.

Re-implementation

Finally I proceeded to re-implement the algorithm. My thought was to simply start at the upper left and swap with bottom right, and move through the entire row. Repeat for next row(s). After doing this I discovered that I had to cover a special case when there was an odd number of rows, so I added a second loop to handle that case. This resulted in the following code:

public void rotateImage180DegIII() {
  for (int x = 0; x < width; x++) {
    for (int y = 0; y < (height / 2); y++) {
      swap(x, y,         width - 1 - x, height - 1 - y);
    }
  }

  if (height % 2 == 1) {
    int mid_y = height / 2;
    for (int x = 0; x < (width / 2); x++) {
      swap(x, mid_y,      width - 1 - x, mid_y);
    }
  }
}

Even without any comments, I think this code reads a lot clearer. The first double for loop reads as swap an element at x, y with an element at width - 1 - x, height - 1 - y. Repeat this for the entire x-column range, and half the height range.

With top left as 0, 0, this reads as swap an element from top left with an element at bottom right. Move to the right for the first element, and to the left for the second element. Repeat until row is swapped, and then repeat for the second (and second to last row).

Finally, if there is an odd number of rows, swap the far left element of the middle row with the far right element on the same row. Increase x and repeat until row is swapped.

Changing dimensions

This code actually handles various dimensions, whereas your original code gets ArrayIndexOutOfBoundsException whenever the second dimension is larger than the first. And it produces strange examples if the second dimension is less than the first.

Here is the output from a 3x3 run (the first line is the Arrays.deepToString(imageMatrix) output):

[[0, 1, 2], [3, 4, 5], [6, 7, 8]]

Original
 0  1  2 
 3  4  5 
 6  7  8 

rotateImage180DegIII
 8  7  6 
 5  4  3 
 2  1  0 

rotateImage180DegII
 8  7  6 
 5  4  3 
 2  1  0 

Nicely rotated by both alternatives. Let's continue with an 5x3 run:

[[0, 1, 2], [3, 4, 5], [6, 7, 8], [9, 10, 11], [12, 13, 14]] 

Original
 0  1  2 
 3  4  5 
 6  7  8 
 9 10 11 
12 13 14 

rotateImage180DegIII
14 13 12 
11 10  9 
 8  7  6 
 5  4  3 
 2  1  0 

rotateImage180DegII
 8  7  6 
 5  4  3 
 2  1  0 
 9 10 11 
12 13 14 

I think my output make sense, but not yours. I'm not sure what is happening, or why. But it does look strange. Finally let us do a 3x5 run:

[[0, 1, 2, 3, 4], [5, 6, 7, 8, 9], [10, 11, 12, 13, 14]]

Original
 0  1  2  3  4 
 5  6  7  8  9 
10 11 12 13 14 

rotateImage180DegIII
14 13 12 11 10 
 9  8  7  6  5 
 4  3  2  1  0 

Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: 4
  at Main.swap(Main.java:13)
  at Main.rotateImage180DegII(Main.java:64)
  at Main.main(Main.java:95)
exited with non-zero status

Lessons to be learned

  • Work on the variable naming – Using x and y which have a very strong co-nation to coordinates, and intermixing them is bad. You loose control over what they indicate.
  • Perform wider tests – I don't think you've tested with alternating dimensions, as you then would have discovered the issues I point out in the Changing dimensions tests
  • Add helper methods – Using helper methods can both help in visualizing the process, and reduce the clutter so you can focus on the algorithm. Compare your original code with the code using swap. Just that is a nice improvement. Adding something like a printMatrix and resetMatrix can also be very helpful to understand what is happening.

Translating my alternate implementation to yours with your definition of imageMagic and RGBColor I trust you'll manage. Note that within the swap method the y coordinates needs to be in the first dimension, whilst the x coordinates are in the second dimension. In other words, keep your head straight on when handling dimension indexes.

Update: Changed to non-static methods, which purely was a result of laziness in my test setup based upon the static main() method, and not proper object initialization.

\$\endgroup\$
3
\$\begingroup\$

@holroy answer is excellent, but if you want to go with something that is even more obvious (hence simpler) at first glance, you could do this

<T> void reverseArray(T[] array) {
    for(int i = 0; i < array.length / 2; i++) {
        T temp = array[i];
        array[i] = array[array.length - i - 1];
        array[array.length - i - 1] = temp;
    }
}

void rotateImage180(RGB[][] raster) {
    for (int i = 0; i < raster.length; i++) {
        reverseArray(raster[i]);
    }
    reverseArray(raster);
}

This works, because 180 rotation is same as symmetry around center point.

\$\endgroup\$
  • \$\begingroup\$ I've removed my earlier comments, as I found a way to compile this within the static context I used (had to replace the int[][] with Integer[][] and some other changes). It does seem to work, but this is not simpler in my book, even though it has fewer lines. \$\endgroup\$ – holroy May 13 '17 at 21:33

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.