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!

3
  • 4
    Set it to true initially, and set it to false if you find non-equal consecutive elements. Commented Feb 20, 2018 at 9: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 Commented Feb 20, 2018 at 9:24
  • A boolean cannot possibly be enough to count four occurrences. You need an int, and you need to clear it on inequality. Commented Feb 20, 2018 at 9:46

6 Answers 6

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.

Sign up to request clarification or add additional context in comments.

4 Comments

Or just ditch the variable, if you're going to return early.
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
@BleronQorri Set the length check to 3. Since the last check will be 2+1=3 which is the index for the fourth number
Also of interest if you get into performance: stackoverflow.com/questions/1029992/…
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])

}

1 Comment

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...
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.

3 Comments

so is always a better approach to start with true and set it to false if a certain condition is met?
Not always, depends on the logic behind it of course.
I emboldened in this case to hint that what you start with depends on circumstance.
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.

Comments

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;
}

Comments

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

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.