4
\$\begingroup\$

Looking for review, good practices, optimizations, clean code tips etc.

final class Pixel {

    private final int color;

    public Pixel(int color) {
        this.color = color;
    }

    public int getColor() {
        return color;
    }
}

/**
 * Rotates an image by ninety degrees.
 * 
 * Complexity:
 * O(row * col)
 */
public final class Rotate {

    private Rotate() { }

    /**
     * Rotes the image in anti clockwise direction.
     * 
     * @param   image the image to be rotated.
     * @return  the titled image.
     * @throws  NPE
     */
    public static Pixel[][] rotateAntiClockWise(Pixel[][] image) {
        int col = image.length;
        int row = image[0].length;

        final Pixel[][] rotatedPixel = new Pixel[row][col];

        for (int i = 0; i < image.length; i++) {
            for (int j = 0; j < image[0].length; j++) {
                rotatedPixel[image[0].length -1 -j][i] = image[i][j];
            }
        }
        return rotatedPixel;
    }


    /**
     * Rotates the image in clockwise direction.
     * 
     * @param   image the image to be rotated.
     * @returns the rotated image.
     * @throws  NPE
     */
   public static Pixel[][] rotateClockWise(Pixel[][] image) {
       int col = image.length;
       int row = image[0].length;

       final Pixel[][] rotatedPixel = new Pixel[row][col];

       for (int i = 0; i < image.length; i++) {
           for (int j = 0; j < image[0].length; j++) {
               rotatedPixel[j][image.length -1 - i] = image[i][j];
           }
       }
       return rotatedPixel;
   }


   public static void main(String[] args) {
       //   int[][] m = { {1, 2, 3, 4} ,
       //                {10, 20, 30, 40}};
       Pixel[][] image = new Pixel[2][4];
       image[0][0] = new Pixel(1);
       image[0][1] = new Pixel(2);
       image[0][2] = new Pixel(3);
       image[0][3] = new Pixel(4);

       image[1][0] = new Pixel(10);
       image[1][1] = new Pixel(20);
       image[1][2] = new Pixel(30);
       image[1][3] = new Pixel(40);

       Pixel[][] rotatedAntiClockNinetyDegreeImage = rotateAntiClockWise(image);

       /*
        * Expected: 
        * 
        * 4  40
        * 3  30
        * 2  20
        * 1  10
        * 
        */
       for (int i = 0; i < rotatedAntiClockNinetyDegreeImage.length; i++) {
           for (int j = 0; j < rotatedAntiClockNinetyDegreeImage[0].length; j++) {
               System.out.print(rotatedAntiClockNinetyDegreeImage[i][j].getColor() + " ");
           }
           System.out.println();
       }

       System.out.println("==========================================");


       Pixel[][] rotatedClockWiseNinetyDegreeImage = rotateClockWise(image);

       /*
        * Expected: 
        * 
        * 10  1 
        * 20  2
        * 30  3
        * 40  4  
        * 
        */
       for (int i = 0; i < rotatedClockWiseNinetyDegreeImage.length; i++) {
           for (int j = 0; j < rotatedClockWiseNinetyDegreeImage[0].length; j++) {
               System.out.print(rotatedClockWiseNinetyDegreeImage[i][j].getColor() + " ");
           }
           System.out.println();
       }
    }
}
\$\endgroup\$
6
\$\begingroup\$

Assuming that you will want to do more interesting things with these images later, you should probably come up with a better OOP design. I suggest:

public interface ImageTransformation {
    public Pixel[][] transform(Pixel[][] image);
}

public class Clockwise90Rotation implements ImageTransformation {
    ...
}

public class AntiClockwise90Rotation implements ImageTransformation {
    ...
}

It's practically no work to rearrange the code that way. In return, you gain the flexibility to do things like:

ImageTransformation[] pipeline = new ImageTransformation[] {
    new Clockwise90Rotation(),
    new ContrastEnhancer(),
    new GaussianBlur(/* radius= */ 5)
};
for (ImageTransformation t : pipeline) {
    image = t.transform(image);
}
\$\endgroup\$
5
\$\begingroup\$

Overall, your code is really nice looking! Below are just some basic suggestions.

final class Pixel {

In my opinion, you should always explicitly dictate whether classes are public, private, or protected.

public static Pixel[][] rotateAntiClockWise(Pixel[][] image) {
    int col = image.length;
    int row = image[0].length;

Here, your method assumes that every image is a rectangle of some kind. If you had an image that was, say, a triangle, your code would fail with either a NullPointerException or an IndexOutOfBoundsException at some point (depending on which end of the triangle you started at).

final Pixel[][] rotatedPixel = new Pixel[row][col];

Out of curiosity, is there a reason you make this two-dimensional array final? It doesn't affect the values stored in the arrays, and the final doesn't affect the reference when it's returned from the method, so there doesn't seem to be much value in having it here (unlike its use in your Pixel class, which makes perfect sense).

Pixel[][] rotatedAntiClockNinetyDegreeImage = rotateAntiClockWise(image);

This is of course pretty subjective, but this variable name is really, really verbose, even by Java standards. rotatedAntiClockwise would be sufficient, or even just rotated for these purposes. Just an opinion.

\$\endgroup\$
  • \$\begingroup\$ Is there any such thing as a triangle image? (A two-dimensional array of pixels that is not rectangular) \$\endgroup\$ – Simon Forsberg Jan 15 '14 at 19:33
  • \$\begingroup\$ That seems trivial to prove, does it not? Hard to type in comments without line breaks, but if the following statements are all true: image[0].length = 1; image[1].length = 2; image[2].length = 3; and so on. If you're asking whether or not this exists in the wild, I assume so. I suppose I could be wrong an all images are rectangular with, at best, transparent backgrounds, but such a categorical statement would seem odd to me. \$\endgroup\$ – asteri Jan 15 '14 at 19:38
  • \$\begingroup\$ Well I've always assumed that all images are rectangular in the computer world. They always have a width and a height. I think that introducing non-rectangular images would cause problems in a lot of areas. Of course it's possible to create a non-rectangular two-dimensional array of pixels with code, but does it exist? Is such an array ever serialized to a real image that is readable by standard image viewer applications? \$\endgroup\$ – Simon Forsberg Jan 15 '14 at 19:49
  • \$\begingroup\$ Good point. No idea, truthfully. \$\endgroup\$ – asteri Jan 15 '14 at 19:50

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.