Determining if three numbers are consecutive

Question

This code will determine if three numbers are consecutive for any order they're supplied to the method (any permutation of [n, n+1, n+2] should be accepted). Is there any way to make this code easier for me to write, or make the code more efficient? Any feedback would be most welcome!

I tried testing with 3! combinations of 2, 3, 4, and it seemed to work. I also tried 1, 2, 9, and 1, 0, 1. All cases seem to work.

public static boolean consecutive(int a, int b, int c) {
    if ( a == b || b == c || a == c) {
        return false;
    } else if (((a == b + 1 || a == b - 1) || (a == c + 1 || a == c - 1)) && ((b == c + 1 || b == c - 1)) 
               || ((b == c + 1 || b == c - 1) || (a == b + 1 || a == b - 1)) && (a == c + 1 || a == c - 1))  {
        return true;
    } 
    return false;
}

Answer 1

That's a lot of cases to enumerate. You're basically doing all of the comparisons that a sorting algorithm would do, but you've "unrolled" the loop.

If you don't want to construct an array and sort it, then you could try this: check that the minimum and maximum differ by 2, and that all three numbers are distinct. Math.min() and Math.max() are just conditionals packaged in a more readable form.

public static boolean consecutive(int a, int b, int c) {
    int min = Math.min(a, Math.min(b, c));
    int max = Math.max(a, Math.max(b, c));
    return max - min == 2 && a != b && a != c && b != c;
}

Answer 2

The problem that I have with writing something like the original code is that it is complicated to be sure that it does the right thing in every situation. You've taken three values and perform whopping fifteen comparisons on them. What if you'd missed one? The code would almost work, except it would occasionally (in that one situation) return the wrong result.

A conceptually simple way to handle this is

public static boolean consecutive(int... numbers) {
    Arrays.sort(numbers);

    for (int i = 1; i < numbers.length; i++) {
        if (numbers[i] != numbers[i-1] + 1) {
            return false;
        }
    }

    return true;
}

Sorting saves a lot of logic here. Since we know the order, we can just check the differences directly. If any adjacent numbers are not consecutive, we can return false. If it makes it all the way through, they all must be consecutive.

Note that this also handles other than three numbers. For example, it always returns true if there's zero or one number passed. This may or may not be what you want.

Now we do five manual comparisons (including the comparison of i to numbers.length) to determine that three numbers are consecutive. The sort does more comparisons, but we don't have to worry if we got those correctly. The chances of there being an error in sort that made it through the Java compiler's testing is extremely low.

I find this version much easier to read and verify correctness than the original code. For most cases, the difference in runtime is going to be minimal and unimportant. My ability to write code that looks like this quickly and be sure it works is usually going to be more important. And if you really need blazing speed, there are always options like @200_success offers.

Answer 3

Agreed, sorting is easiest to simplify the code. To guarantee exactly three, you would call something like int numArr = new int[]{8,6,7}; consecutive(3, numArr); using this method:

public static boolean consecutive(int count, int... numbers) {
    if (numbers.length != count) {//make sure the correct number of numbers was sent in
        return false;
    }
    Arrays.sort(numbers);//guarantee order to simplify logic

    for (int i = 1; i < numbers.length; i++) {
        //compare this num to the previous num, make sure its exactly 1 greater
        if (numbers[i] != numbers[i - 1] + 1) {
            return false;
        }
    }
    return true;
}

Answer 4

Your code is properly formatted (except for one superfluous space), however, the lengthy expression

((a == b + 1 || a == b - 1) || (a == c + 1 || a == c - 1)) && ((b == c + 1 || b == c - 1)) 
|| ((b == c + 1 || b == c - 1) || (a == b + 1 || a == b - 1)) && (a == c + 1 || a == c - 1)

is pretty hard to read. The best solution is to use either sorting or something smart as 200_success or rolfl proposed. I try an advice for the case that no such solution is available.

---

Note that you're doing something like

if (x) {
     return false;
} else if (y) {
     return true;
}
return false;

This is actually always sort of wrong as you can do

if (x) {
     return false;
} else {
     return y;
}

instead. You can also leave out the "else", but that's matter of style.

---

As your lengthy condition is a disjunction and the action is trivial, you can simply split it like

if (x) {
     return false;
} else if (y1) {
     return true;
} else if (y2) {
     return true;
}
return false;

Here, I avoided my above simplification to preserve symmetry.

---

Let's look at the part denoted as y1, i.e.,

((a == b + 1 || a == b - 1) || (a == c + 1 || a == c - 1)) && ((b == c + 1 || b == c - 1)) 

now. The first part states that the distance of a and b is 1, in other words Math.abs(a - b) == 1. You can use it to make your conditions slightly less repetitive.

---

More importantly, observe the same expression appear later again, define some local variables to keep it short (with or without abs; the idea is independent):

if (a == b || b == c || a == c) {
    return false;
}

boolean ab = a == b + 1 || a == b - 1;
boolean bc = b == c + 1 || b == c - 1;
boolean ac = a == c + 1 || a == c - 1;

return (ab | ac) & bc || (bc | ab) & ac;

My above naming is not the best, however, I consider it acceptable as the scope is very limited. Still, because of the asymmetry, it's a bit hard to tell if it's right.

You could rewrite it as

return ab & bc | ab & ac | bc & ac;

---

Another simplification is possible by observing that shifting all value by the same distance doesn't change anything. So you could do something like

a -= c;
b -= c;
c -= c; // i.e., c = 0

and simplify the other expression a bit. This isn't worth it here. By using the arithmetic, you make yourself susceptible to overflow just like here.

---

To add something funny, I propose this

public static boolean consecutive(int a, int b, int c) {
    return 2 ==
        + (Math.abs(a - (long) b) == 1 ? 1 : 0)
        + (Math.abs(b - (long) c) == 1 ? 1 : 0)
        + (Math.abs(a - (long) c) == 1 ? 1 : 0);
}

It works simply by requiring exactly two of the distances to equal one. The case to long prevents overflow (and may be left out if you don't mind wrapping around Integer.MAX_VALUE).

Answer 5

A recursive solution:

public static boolean consecutive(int a, int b, int c) {
    switch (Math.abs(c-a)) {
        case 2: return ( 2*b == a+c );     // b should be right between a and c
        case 1: return consecutive(b,c,a); // try another pair of outer numbers
        default: return false;
    }
}

The check 2*b == a+c should work fine even in case of integer overflow (not sure about false positives, though).

Answer 6

Observing that a and b must be close together, this code is not too bad:

public static boolean consecutive(int a, int b, int c) {
    switch (b - a) {
        case +1: return c == b + 1 || c == a - 1;
        case +2: return c == a + 1;
        case -1: return c == a + 1 || c == b - 1;
        case -2: return c == b + 1;
        default: return false;
    }
}

And an alternative:

public static boolean consecutive(int a, int b, int c) {
    return (  b < a ? consecutive (b, a, c)
            : b == a + 1 ? (c == b + 1 || c == a - 1)
            : b == a + 2 ? (c == a + 1)
            : false);
}

Answer 7

I prefer to break my code down into modules:

public boolean threeConsecutive (int n1, int n2, int n3) {
    int nConsecutivePairs = differ(n1 - n2, 1) + differ(n2 - n3, 1) + differ(n3 - n1, 1); 
    int nSecondPairs = differ(n1 - n2, 2) + differ(n2 - n3, 2) + differ(n3 - n1, 2);
    return (nConsecutivePairs == 2 && nSecondPairs == 1);
}
private int differ(int n1, int n2) {
if (n1 == n2 || (n1 + n2) == 0)
    return 1;
else 
    return 0;
}

Answer 8

One possibility would be to use a Set in order to check for duplicate integers. If there are no duplicates, the length of the list should be max - min + 1:

public static boolean areConsecutiveIntegers(Integer[] numbers) {
    List<Integer> numbersList = Arrays.asList(numbers);
    Integer min = Collections.min(numbersList);
    Integer max = Collections.max(numbersList);
    Set<Integer> uniqueNumbers = new HashSet<>(numbersList);
    return numbersList.size() == uniqueNumbers.size() && numbersList.size() == max - min + 1;
}

Here are some tests:

package stackoverflow;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.junit.Test;


public class ConsecutiveIntegers
{
    @Test
    public void testConsecutiveIntegers() {
        Integer[][] consecutives = {
                { 1, 2, 3 },
                { 3, 2, 1 },
                { 1, 3, 2 },
                { 1, 2, 3, 5, 6, 7, 4, 0, 9, 8, -1 },
                { 1 },
        };

        Integer[][] nonConsecutives = {
                { 2, 0 },
                { 1, 2, 3, 3 },
                { 1, 2, 2 },
                { -1, 1 },
                { 1, 2, 3, 5, 6, 7, 4, 9, 8, -1 },
        };

        for (int i = 0; i < consecutives.length; i++) {
            assertTrue(areConsecutiveIntegers(consecutives[i]));
        }

        for (int i = 0; i < nonConsecutives.length; i++) {
            assertFalse(areConsecutiveIntegers(nonConsecutives[i]));
        }
    }

    public static boolean areConsecutiveIntegers(Integer[] numbers) {
        List<Integer> numbersList = Arrays.asList(numbers);
        Integer min = Collections.min(numbersList);
        Integer max = Collections.max(numbersList);
        Set<Integer> uniqueNumbers = new HashSet<>(numbersList);
        return numbersList.size() == uniqueNumbers.size() && numbersList.size() == max - min + 1;
    }
}