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

Rotating an array by position n

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

Problem

Input is 1,2,3,4,5,6,7,8


If rotateOrder = 2


Output should be 7,8,1,2,3,4,5,6


If rotateOrder = 3


Output should be 6,7,8,1,2,3,4,5

Here is my program to do it:

int [] unOrderedArr = {1,2,3,4,5,6,7,8};
        int  orderToRotate =2;

       for(int i = 0; i0; j--){
               int temp = unOrderedArr[j];
               unOrderedArr[j] = unOrderedArr[j-1];
               unOrderedArr[j-1] = temp;

           }

        }

       for(int j = 0; j<unOrderedArr.length; j++){
         System.out.println("element is " + unOrderedArr[j]);

           }


How can it be optimized?

Solution

Validate your inputs

The current method doesn't check for invalid input:

  • null array



  • negative rotation order



I suggest to add this ad the top:

if (arr == null || order < 0) {
    throw new IllegalArgumentException("The array must be non-null and the order must be non-negative");
}


Follow standard formatting

The posted code is messy. Use your IDE to reformat it to the standard:

int[] unOrderedArr = {1, 2, 3, 4, 5, 6, 7, 8};
int orderToRotate = 2;

for (int i = 0; i  0; j--) {
        int temp = unOrderedArr[j];
        unOrderedArr[j] = unOrderedArr[j - 1];
        unOrderedArr[j - 1] = temp;
    }
}


Now that's a lot easier to read.

On a related note, an easy way to print an array is using Arrays.toString

Optimize

Frist of all,
what will happen if you receive arr to rotate, and the rotate order = arr.length?
The array elements will be rotated pointlessly,
just to return to their original position.
This gets even worse for multiples of arr.length.

Secondly, in this code:

for (int i = 0; i  0; j--) {
        // ...
    }
}


The above code iterates over the entire array order times.

To avoid unnecessary iterations,
you will benefit from using a clone of the original array,
and iterate over the elements only once,
at the expense of using twice as much memory.

Suggested implementation

The above suggestions and some style improvements applied:

private static void rotate(int[] arr, int order) {
    if (arr == null || order  0) {
        int[] copy = arr.clone();
        for (int i = 0; i < arr.length; ++i) {
            int j = (i + offset) % arr.length;
            arr[i] = copy[j];
        }
    }
}


Some unit test to verify it works:

@Test
public void testRotateBy2() {
    int[] arr = {1, 2, 3, 4, 5, 6, 7, 8};
    int[] expected = {7, 8, 1, 2, 3, 4, 5, 6};
    rotate(arr, 2);
    assertArrayEquals(expected, arr);
}

@Test
public void testRotateBy3() {
    int[] arr = {1, 2, 3, 4, 5, 6, 7, 8};
    int[] expected = {6, 7, 8, 1, 2, 3, 4, 5};
    rotate(arr, 3);
    assertArrayEquals(expected, arr);
}

@Test
public void testRotateByLength() {
    int[] arr = {1, 2, 3, 4, 5, 6, 7, 8};
    int[] expected = arr.clone();
    rotate(arr, arr.length);
    assertArrayEquals(expected, arr);
    rotate(arr, arr.length * 3);
    assertArrayEquals(expected, arr);
}

@Test
public void testRotateByZero() {
    int[] arr = {1, 2, 3, 4, 5, 6, 7, 8};
    int[] expected = arr.clone();
    rotate(arr, 0);
    assertArrayEquals(expected, arr);
}

Code Snippets

if (arr == null || order < 0) {
    throw new IllegalArgumentException("The array must be non-null and the order must be non-negative");
}
int[] unOrderedArr = {1, 2, 3, 4, 5, 6, 7, 8};
int orderToRotate = 2;

for (int i = 0; i < orderToRotate; i++) {
    for (int j = unOrderedArr.length - 1; j > 0; j--) {
        int temp = unOrderedArr[j];
        unOrderedArr[j] = unOrderedArr[j - 1];
        unOrderedArr[j - 1] = temp;
    }
}
for (int i = 0; i < order; i++) {
    for (int j = arr.length - 1; j > 0; j--) {
        // ...
    }
}
private static void rotate(int[] arr, int order) {
    if (arr == null || order < 0) {
        throw new IllegalArgumentException("The array must be non-null and the order must be non-negative");
    }
    int offset = arr.length - order % arr.length;
    if (offset > 0) {
        int[] copy = arr.clone();
        for (int i = 0; i < arr.length; ++i) {
            int j = (i + offset) % arr.length;
            arr[i] = copy[j];
        }
    }
}
@Test
public void testRotateBy2() {
    int[] arr = {1, 2, 3, 4, 5, 6, 7, 8};
    int[] expected = {7, 8, 1, 2, 3, 4, 5, 6};
    rotate(arr, 2);
    assertArrayEquals(expected, arr);
}

@Test
public void testRotateBy3() {
    int[] arr = {1, 2, 3, 4, 5, 6, 7, 8};
    int[] expected = {6, 7, 8, 1, 2, 3, 4, 5};
    rotate(arr, 3);
    assertArrayEquals(expected, arr);
}

@Test
public void testRotateByLength() {
    int[] arr = {1, 2, 3, 4, 5, 6, 7, 8};
    int[] expected = arr.clone();
    rotate(arr, arr.length);
    assertArrayEquals(expected, arr);
    rotate(arr, arr.length * 3);
    assertArrayEquals(expected, arr);
}

@Test
public void testRotateByZero() {
    int[] arr = {1, 2, 3, 4, 5, 6, 7, 8};
    int[] expected = arr.clone();
    rotate(arr, 0);
    assertArrayEquals(expected, arr);
}

Context

StackExchange Code Review Q#69299, answer score: 13

Revisions (0)

No revisions yet.