10
\$\begingroup\$

I tried to do it by using ArrayUtils.reverse() but I couldn't figure out how to compare this with the other array. So I wrote this instead:

public class Main {

  public static void main(String[] args) {

int[] array = new int[] {1, 2, 3, 4, 5, 6};

int[] tempArray = new int[] {6, 5, 4, 3, 2, 1};

boolean result = true;

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

for (int i = 0; i < array.length; i++) {
  if (array[i] != tempArray[i]) {
    System.out.println("nope,not reversed");
    result = false;
    break;
  }
}

if (result) {
  System.out.println("yep,reversed");
}
  }
}

Is there a simpler solution? I'm skeptic because I think breaking the loop and setting a boolean value is a bit unnecessary.

\$\endgroup\$
  • \$\begingroup\$ What is ArrayUtils.reverse()? \$\endgroup\$ – Tunaki Mar 6 '17 at 18:48
  • \$\begingroup\$ @Tunaki commonLangs class. It's a bug-safe method to reverse arrays ArrayUtils.reverse(int[] array) \$\endgroup\$ – Egek92 Mar 6 '17 at 18:54
  • 2
    \$\begingroup\$ Why do you need to reverse it to compare? Just compare to the reverse. Show indents. \$\endgroup\$ – paparazzo Mar 6 '17 at 19:23
17
\$\begingroup\$

The first comment, is that it would be preferable to factor this into a dedicated method, like areReversed(int[] array1, int[] array2). It would clarify the intention of the code. And, as it turns out, this will point out some improvements about the method itself.

  • It is currently modifying in-place the first array, in order to reverse it. If you now consider the code to be in a method, you could have

    int[] array1 = new int[] { 1, 2, 3, 4, 5, 6 };
    int[] array2 = new int[] { 6, 5, 4, 3, 2, 1 };
    areReversed(array1, array2); // OK, prints that they are reversed versions
    // some code
    areReversed(array1, array2); // What? Now it prints that they aren't reversed?!
    

    Calling the method a second time with the same arguments would not have the same result. This would be very surprising, especially so that one doesn't expect that method to behave in such a way. It is always preferable to adhere to the principle of least astonishment.

  • We want to know whether the two arrays are reversed, and we may want to do more than printing a String if it is the case. Make the method return a boolean that the calling code can use.

The insight is that you do not need to reverse an array, and determine whether the two are equal, for this. First of all, if they don't have the same length, there is no need to do anything, it is already known for sure that the result is false. Then, you can directly compare the arrays, by traversing one in a given direction and the other one in the opposite direction.

B   A   C            B   A   C            B   A   C
|                        |                        |
----x----                x                ----x----
        |                |                |
C   A   B            C   A   B            C   A   B

 Step 1               Step 2                Step 3

The upper part goes from left to right, and it compares elements with the lower part going from right to left. If there's a mismatch at any point, we know the arrays are not reverse of each other. And since we know they have the same length, all element of both arrays will have been considered.

A simple code to do this would be

public boolean areReversed(int[] array1, int[] array2) {
    if (array1.length != array2.length) {
        return false;
    }
    int length = array1.length;
    for (int i = 0; i < length; i++) {
        if (array1[i] != array2[length - i - 1]) {
            return false;
        }
    }
    return true;
}

That you can later use with:

if (areReversed(array, tempArray)) {
    System.out.println("yep,reversed");
} else {
    System.out.println("nope,not reversed");
}

You'll note that this implementation somewhat takes care of the concern you had with the code: "breaking the loop and setting a boolean value". In fact, there is no boolean variable: we return directly from the method once we know for sure what to return. The return inside the loop is really for optimization purposes; once a mismatch is encountered, there is no need to process further the arrays, since it is known that the result can only be false at that point. This is also what the initial break was doing.


Small final note. Instead of

int[] array = new int[] { 1, 2, 3, 4, 5, 6 };
int[] tempArray = new int[] { 6, 5, 4, 3, 2, 1 };

you could just have

int[] array = { 1, 2, 3, 4, 5, 6 };
int[] tempArray = { 6, 5, 4, 3, 2, 1 };

which is a bit more concise, and makes the array declaration a tad more readable.

\$\endgroup\$
  • \$\begingroup\$ I would think the iterations could be cut in half. One such way may be to compare (i > length-i). I would think that is proof enough all elements have been checked. \$\endgroup\$ – Enigma Maitreya Mar 6 '17 at 20:39
  • \$\begingroup\$ Yes @Enigma good point, you could enhance a little and only traverse half, if you add || array1[length - i - 1] != array2[i] in the condition. \$\endgroup\$ – Tunaki Mar 6 '17 at 20:48
  • \$\begingroup\$ I don't know about the OP but I would appreciate you modifying your code. JAVA is not one of my comp languages and find it useful to browse through these questions so I can ... so to say get a better reading java comprehension by seeing code that I can then mentally translate to a language(s) do know. If it is a problem I understand. I modified my answer to incorporate what I think should work. BTW +1 for the completeness of your review. (and dedicated method comment) \$\endgroup\$ – Enigma Maitreya Mar 6 '17 at 20:54
  • \$\begingroup\$ Bah, my deepest apologies for leading you astray excellent answer on your part. \$\endgroup\$ – Enigma Maitreya Mar 7 '17 at 0:31
  • 1
    \$\begingroup\$ Another possibility for the loop: for (int i = 0, j = length-1; i <= j; i++,j--) \$\endgroup\$ – NonlinearFruit Mar 7 '17 at 13:13
2
\$\begingroup\$

I think @Enigma Maitreya already touched the most important points, I would add a null check on both params to prevent the code to throw an NPE, so instead of this:

if (array1.length != array2.length) {
        return false;
}

... it would be like:

if (array1 == null || array2 == null) {
    throw new Exception("array1 & array2 are required");
}

if (array1.length != array2.length) {
        return false;
}
\$\endgroup\$
0
\$\begingroup\$

I hope I have the java coding on this right :( NOTE: Looking at another (Tunaki) answer, this is ... in my opinion effectively the same answer ... just using a do vs a for.

// Initial determination of false
if (Array.Length != tempArray.length) {
   return false;
}
// Lets check the arrays
i = 0;
do {
    if (array(i) != tempArray(array.Length - i - 1)) {
        return false;
   }
// This was proven wrong   } while (++i <= (array.Length-i)); 
// The above should check all elements to the middle then end
} while (++i <= (array.Length-1)); 

return true;

I believe I have not introduced any additional work being done and think I have removed the initial for loop work.

This has been proven wrong

Improvement would be to check to the middle but that implies the length = even.

It still can be done just need to accept odds (rounds high) an additional compare at the beginning ([0] to [length-1] or (rounds low) a check at the end [i] to [i+1] for a middle comparison. Sorry should add the i begins at 1 when the length is odd.

Astute observation as was made

Could you elaborate why checking to only the middle in sufficient? Let's consider the example {1, 2, 3} vs {3, 2, 4}. Checking till middle would fail here, wouldn't it? – Herickson

Let us assume sequence 123456789 compared to 9876545321 Iterations:

1 1=1 (0 to 9)

2 2=2 (1 to 8)

3 3=3 (2 to 7)

4 4=4 (3 to 6)

5 5=5 (4 to 5) - midpoint

6 6=6 (5 to 4)

7 7=7 (6 to 3)

8 8=8 (7 to 2)

9 9=9 (8 to 1)

So Proven there is NO correctness to this. So I will correct my mistake and thank Herickson for his astute observation :)

\$\endgroup\$
  • 1
    \$\begingroup\$ Could you elaborate why checking to only the middle in sufficient? Let's consider the example arrays {1, 2, 3} vs {4, 2, 1}. Checking till middle would fail here, wouldn't it? \$\endgroup\$ – user101048 Mar 7 '17 at 0:13
  • 1
    \$\begingroup\$ @Herickson - Indeed failure is guaranteed. I have corrected the answer to demonstrate the failure. \$\endgroup\$ – Enigma Maitreya Mar 7 '17 at 0:29
  • \$\begingroup\$ Thank you :) Minor note: You cite my comment with the arrays {1, 2, 3} and {3, 2, 4}. You probably caught my comment in an edit here. Of course {1, 2, 3} and {4, 2, 1} are the correct arrays. \$\endgroup\$ – user101048 Mar 7 '17 at 0:33
-1
\$\begingroup\$

We will use your same idea, just simplify your code.

In your first loop you do

int temp = array[i]; 
array[i] = array[array.length - i - 1];   //1
array[array.length - i - 1] = temp;     //2

And then, in the second loop

if (array[i] != tempArray[i])

But by //1 and //2 we can omit the first loop and in the second we change array[i] by array[array.length - i - 1], so we have

boolean result = true;
s = array.length;
if( tempArray.length != s)
    result = false;
i = 0;
while (i < tam/2 and result != false )
    if (array[i] != tempArray[tam-i-1])
        result = false;
if ( result ) 
    System.out.println("yep, reversed");
else    
    System.out.println("nope,not reversed");
\$\endgroup\$
  • 2
    \$\begingroup\$ Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please explain your reasoning (how your solution works and why it is better than the original) so that the author and other readers can learn from your thought process. More so this is not compilable Java. \$\endgroup\$ – Marc-Andre Mar 6 '17 at 19:09
  • 1
    \$\begingroup\$ You are right, that is unclear. I will change it \$\endgroup\$ – EmmanuelAC Mar 6 '17 at 20:03

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.