0

I'm trying to develop a method that checks whether first 4 elements of a given array (assuming the array has at least 4 elements) are consecutively equal to one another, my first approach was to create a boolean variable and set it as false at first and if a[i] == a[i + 1] set it to true. But the problem is that whether the 4 elements are or are not consecutive it always prints it as true.

public static boolean isConsecutive(int[] a) {
    boolean isConsecutive = false;
    for (int i = 0; i < 4; i++) {
        if (a[i] == a[i + 1]) {
            isConsecutive = true;
        }
    }
    return isConsecutive;
}

Where is my mistake? Thanks in advance!

  • 4
    Set it to `true` initially, and set it to `false` if you find non-equal consecutive elements. – Andy Turner Feb 20 '18 at 09:24
  • Indeed, a single match will set it to true, while your goal is to set it to false if you find one case where it doesn't match – Stultuske Feb 20 '18 at 09:24
  • A boolean cannot possibly be enough to count four occurrences. You need an `int`, and you need to clear it on inequality. – user207421 Feb 20 '18 at 09:46

6 Answers6

2

You need a else branch to set it to false again if it was not consecutive and loop until 3. Or preferable a return statement as soon as something is not equal e.g.

if (a[i] == a[i + 1]) {
    isConsecutive = true;
} else {
    return false;
}

You could also dismiss the variable isConsecutive something like

 public static boolean isConsecutive(int[] a) {
     for (int i = 0; i < 3; i++) {
         if (!(a[i] == a[i + 1])) {
             return false;
         }
     }
     return true;
 }

Mind that your loop is not safe for index out of bond exception since it may have a size lower than 4.

Murat Karagöz
  • 35,401
  • 16
  • 78
  • 107
  • Or just ditch the variable, if you're going to return early. – Andy Turner Feb 20 '18 at 09:24
  • I tried modifying your code along with this model but the problem still persists, int a[]= {1,1,1,1,2,3,2,1,3,4}; for this array it is returning it as false, I cannot comprehend why –  Feb 20 '18 at 09:32
  • @BleronQorri Set the length check to `3`. Since the last check will be `2+1=3` which is the index for the fourth number – Murat Karagöz Feb 20 '18 at 09:34
  • Also of interest if you get into performance: https://stackoverflow.com/questions/1029992/is-the-inequality-operator-faster-than-the-equality-operator – Christophe Roussy Feb 20 '18 at 09:45
2

using your approach our mine approach you may get arrayOutOfBounds Exception. By the way, i think this approach bellow is easier.

public static boolean isConsecutive(int[] a) {

    return (a[0] == a[1] && 
            a[0] == a[2] &&
            a[0] == a[3])

}
  • what about `a[0]` ? Other than this, assuming the array is at least of size 4, this is the optimal low level solution as it unrolls the loop :), not sure if it matters a lot in Java though... – Christophe Roussy Feb 20 '18 at 09:47
1

Your code sets isConsecutive to false and aims to set it to true when it finds evidence. The problem is that a[i]==a[i+1] is only partial evidence. So you set it to true when a[0] == a[1], and never change it back if (say) a[1] != a[2].

In this case, it would work better if you start with isConsecutive = true, and in your loop, look for conclusive evidence that it's false:

boolean firstFourEqual(int[] a) {
    boolean isConsecutive=true;
    for(int i=0;i<3;i++) {
        if(a[i]!=a[i+1]) {
            isConsecutive=false;
        }
    }
    return isConsecutive;
}

(note that we're iterating 3 times, not 4, because the third test checks the third and fourth element.)

Once you've set isConsecutive to false, there's no going back, so you might as well leave the loop. Two ways you could do this:

boolean firstFourEqual(int[] a) {
    boolean isConsecutive=true;
    for(int i=0;i<3;i++) {
        if(a[i]!=a[i+1]) {
            isConsecutive=false;
            break;                <-- leave the loop early
        }
    }
    return isConsecutive;
}

Or:

boolean firstFourEqual(int[] a) {
    for(int i=0;i<3;i++) {
        if(a[i]!=a[i+1]) {
            return false;
        }
    }
    return true;
}

Some people think having a return mid-method is bad style. Others think that having a mutable variable is bad style. My preference is for the last of these examples. The method is short and clear. There's a mid-method return, but the method is so short, it's easy to spot.


It's not always best to start with true and amend to false, or vice versa. However, in a situation like this where a hypothesis (e.g. "The first four items are equal") can be instantly disproved by a single piece of evidence ("False because items 3 and 4 are unequal"), it makes sense to default to the result of the hypothesis, changing it to the opposite result if you find evidence otherwise.


Finally, you could do this with streams:

 return IntStream.range(0,3)
    .mapToObj( x -> a[x] == a[x+1])
    .noneMatch( b -> b == false);

All of these assume that the array has as size of at least 4. You would need more code if this can't be guaranteed.

slim
  • 40,215
  • 13
  • 94
  • 127
0

Your isConsecutive variable is set to false outside the for loop.

public static boolean isConsecutive(int[] a) {
        **boolean isConsecutive=false**;
        for(int i=0;i<4;i++) {
            if(a[i]==a[i+1]) {
                isConsecutive=true;
            }
        }


        return isConsecutive;
    }

That is why the variable is set to true during the first few iterations when the consecutive variables are equal and then the value of the variable is never changed.

Instead do this:-

public static boolean isConsecutive(int[] a) {

        for(int i=0;i<4;i++) {
        boolean isConsecutive=false;    
        if(a[i]==a[i+1]) {
                isConsecutive=true;
            }
        }


        return isConsecutive;
    }

But this is not an ideal thing to do. The best approach will be to add an else condition where the value of the variable is changed when the if condition is not true.

public static boolean isConsecutive(int[] a) {
        boolean isConsecutive=false;
        for(int i=0;i<4;i++) {

        if(a[i]==a[i+1]) {
                `isConsecutive=true;`
            }
        else {
    isConsecutive=true;
        }
        }



        return isConsecutive;
    }

By doing this you again change the value of the variable when the consecutive variables are not equal.

0

The Problem is with you if condition. It is not checking for 4 consecutive similar elements instead it is checking for two consecutive similar elements.

if(a[i]==a[i+1]) {
            isConsecutive=true;
        }

If your array is [1,2,2,3,4,5,6] ,In this array when your function will check a[1]==a[2], It will set isConsecutive to true and It will remain true for complete iteration of array.

So as you mentioned that array has at least 4 elements so you write you function like

 public static boolean isConsecutive(int[] a) {

         if(a[0]==a[1] && a[0]==a[2] && a[0]==a[3])
              return true;
         else
              return false;
}

And If you want to check 4 consecutive elements anywhere in the array then write your function

public static boolean isConsecutive(int[] a) {
    boolean isConsecutive=false;
    for(int i=0;i<a.length-3;i++) {
        if(a[i]==a[i+1] && a[i]==a[i+2] && a[i]==a[i+3])) {
            isConsecutive=true;
        }
    }

    return isConsecutive;
}
Manoj Kumar Dhakad
  • 1,862
  • 1
  • 12
  • 26
0

Once you've set the isConsecutive variable value to true in a certain i value (e.g. 1), the value will always be true, even if in the next iteration (when i == 2), the value of the next element (a[3]) is different than current element (a[2]).

Also, since the limit in the for-loop is 4, it performs the checking of the 4th and 5th element as well. It will also throw ArrayIndexOutOfBoundsException when there are only 4 elements in the array.

You need to do the following things:

  1. Change back the isConsecutive value to false once there's a difference between a current element and the next element
  2. Set the limit bound to 3, so that the last iteration won't check the 5th element
  3. Add the boolean flag checking in the for-loop termination condition as well; therefore, if the 2nd element is already different to the 1st element, the loop will stop:

    public static boolean isConsecutive(int[] a) {
        boolean different = false;
        for (int i = 0; i < 3 && !different; i++) {
            different = (a[i + 1] != a[i]);
        }
        return !different;
    }
    

PS: I rename isConsecutive boolean to different for better readability