3
\$\begingroup\$

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

/**
 * Flip the columns.
 * 
 * Complexity:
 * O(row * col)
 * 
 */
public final class Mirror {

    private Mirror () { }

    /**
     * Given a matrix create a mirror image,
     * 
     * @param m     the input matrix
     * @throws      NPE if exception occurs.
     */
    public static void mirrorPatch(int[][] m) {
        // for each row.
        for (int i = 0; i < m.length; i++) {
            // for each column
            flipRow(m[i]);
        }
    }

    private static void flipRow(int[] row) {
        int length = row.length;
        // simple swap of each element.
        for (int i = 0; i < length/2; i++) {
            int x = row[i];
            row[i] = row[length -1 - i];
            row[length -1 - i] = x;
        }
    }

    public static void main(String[] args) {
        // even number of columns.
        int[][] m = { {1, 2, 3, 4} , {10, 20, 30, 40}};
        mirrorPatch(m);

        /*
         * Informally verifying that output is:
         * 4   3   2   1  
         * 40  30  20  10
         * 
         */
        for (int i = 0; i < m.length; i++) {
            for (int j = 0; j < m[0].length; j++) {
                System.out.print(m[i][j] + " ");
            }
            System.out.println();
        }

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

        // odd number of columns.

        /*
         * Informally verifying that output is:
         *  5   4   3   2   1
         *  50  40  30  20  10
         */
        int[][] m1 = { {1, 2, 3, 4, 5} , {10, 20, 30, 40, 50}};
        mirrorPatch(m1);

        for (int i = 0; i < m1.length; i++) {
            for (int j = 0; j < m1[0].length; j++) {
                System.out.print(m1[i][j] + " ");
            }
            System.out.println();
        }

        System.out.println("-----------------------------------");
    }
}
\$\endgroup\$
2
\$\begingroup\$

In your top=level method mirrorPatch you have:

    // for each row.
    for (int i = 0; i < m.length; i++) {
        // for each column
        flipRow(m[i]);
    }

This is unnecessarily verbose, using the iterable nature of arrays you could simply:

    for (int[] row : m) {
        flipRow(row);
    }

This is self-documenting, and all good.

In your flipRow method, you do a bit more work, and, it is pretty well structured, and readable. If you want to perhaps squeeze out some more performance, I can suggest some changes. Specifically, since you are creating the length variable, you may as well make it more useful by changing it to be last instead, which simplifies the indexing a little bit. Additionally, sometimes (when it is possible), a subtracting-loop is faster (because the condition is simpler), consider the changes as follows:

private static void flipRow(final int[] row) {
    final int last = row.length - 1;
    for (int i = last/2; i >= 0; i--) {
        int x = row[i];
        row[i] = row[last - i];
        row[last - i] = x;
    }
}
\$\endgroup\$
2
\$\begingroup\$

Since you never instantiate the Mirror class the private constructor private Mirror () { } is unnecessary.

Another approach if your matrix is sufficiently big and you don't actually want to flip it's memory representation is to encapsulate the data in your Mirror class and expose and iterator and other methods to access the data by index which automatically show the matrix as flipped while not actually flipping the class internal representation of your data.

You could put your test into proper unit tests. That way you can leave the task of comparing the expected result with the result of your code to the computer.

You should leave out comments which describe what the code does. E.g. // for each row. , // for each column or // simple swap of each element.. This can be understood from your code just by looking at it, especially if you use sufficiently descriptive variable and method names. Comments should be used to describe things which are not obvious to the reader of your code like why you used one approach over the other.

\$\endgroup\$

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.