Skip to main content
added 305 characters in body
Source Link
Roland Illig
  • 21.9k
  • 2
  • 36
  • 83

You don't need a counter in your code:

public boolean isConsecutive(ArrayList<Integer> list) {
    for (int i = 1;0; i < list.size(); - 1; i++) {
        if (list.get(i - 1) + 1 != list.get(i + 1) - 1) {
            return false;
        }
    }
    return true;
}

I also changedAs a next step, you can change the loop to start at 1 instead of 0, so that the loop condition becomes simpler.

public boolean isConsecutive(ArrayList<Integer> list) {
    for (int i = 1; i < list.size(); i++) {
        if (list.get(i - 1) + 1 != list.get(i)) {
            return false;
        }
    }
    return true;
}

The specification does not require the algorithm to be constant-time (it's not used for cryptography), therefore it should be OK to return as soon as you know the answer.

To be extra correct, you could detect integer overflows:

int prev = list.get(i - 1);
if (prev != list.get(i) /* mismatch */ || prev + 1 < prev || prev/* !=overflow list.get(i)*/) {
    return false;
}

If the requirements were to change that you may only access each list element once, you would need to remember the previous value.

public boolean isConsecutive(ArrayList<Integer> list) {
    Iterator<Integer> it = list.iterator();
    if (!it.hasNext()) {
        return true;
    }

    int prev = it.next();
    while (it.hasNext()) {
        int curr = it.next();
        if (prev + 1 != curr /* mismatch */ || prev + 1 < prev /* overflow */) {
            return false;
        }
        prev = curr;
    }
    return true;
}

You don't need a counter in your code:

public boolean isConsecutive(ArrayList<Integer> list) {
    for (int i = 1; i < list.size(); i++) {
        if (list.get(i - 1) + 1 != list.get(i)) {
            return false;
        }
    }
    return true;
}

I also changed the loop to start at 1 instead of 0, so that the loop condition becomes simpler.

The specification does not require the algorithm to be constant-time (it's not used for cryptography), therefore it should be OK to return as soon as you know the answer.

To be extra correct, you could detect integer overflows:

int prev = list.get(i - 1);
if (prev + 1 < prev || prev != list.get(i)) {
    return false;
}

If the requirements were to change that you may only access each list element once, you would need to remember the previous value.

public boolean isConsecutive(ArrayList<Integer> list) {
    Iterator<Integer> it = list.iterator();
    if (!it.hasNext()) {
        return true;
    }

    int prev = it.next();
    while (it.hasNext()) {
        int curr = it.next();
        if (prev + 1 != curr /* mismatch */ || prev + 1 < prev /* overflow */) {
            return false;
        }
        prev = curr;
    }
    return true;
}

You don't need a counter in your code:

public boolean isConsecutive(ArrayList<Integer> list) {
    for (int i = 0; i < list.size() - 1; i++) {
        if (list.get(i) != list.get(i + 1) - 1) {
            return false;
        }
    }
    return true;
}

As a next step, you can change the loop to start at 1 instead of 0, so that the loop condition becomes simpler.

public boolean isConsecutive(ArrayList<Integer> list) {
    for (int i = 1; i < list.size(); i++) {
        if (list.get(i - 1) + 1 != list.get(i)) {
            return false;
        }
    }
    return true;
}

The specification does not require the algorithm to be constant-time (it's not used for cryptography), therefore it should be OK to return as soon as you know the answer.

To be extra correct, you could detect integer overflows:

int prev = list.get(i - 1);
if (prev != list.get(i) /* mismatch */ || prev + 1 < prev /* overflow */) {
    return false;
}

If the requirements were to change that you may only access each list element once, you would need to remember the previous value.

public boolean isConsecutive(ArrayList<Integer> list) {
    Iterator<Integer> it = list.iterator();
    if (!it.hasNext()) {
        return true;
    }

    int prev = it.next();
    while (it.hasNext()) {
        int curr = it.next();
        if (prev + 1 != curr /* mismatch */ || prev + 1 < prev /* overflow */) {
            return false;
        }
        prev = curr;
    }
    return true;
}
Source Link
Roland Illig
  • 21.9k
  • 2
  • 36
  • 83

You don't need a counter in your code:

public boolean isConsecutive(ArrayList<Integer> list) {
    for (int i = 1; i < list.size(); i++) {
        if (list.get(i - 1) + 1 != list.get(i)) {
            return false;
        }
    }
    return true;
}

I also changed the loop to start at 1 instead of 0, so that the loop condition becomes simpler.

The specification does not require the algorithm to be constant-time (it's not used for cryptography), therefore it should be OK to return as soon as you know the answer.

To be extra correct, you could detect integer overflows:

int prev = list.get(i - 1);
if (prev + 1 < prev || prev != list.get(i)) {
    return false;
}

If the requirements were to change that you may only access each list element once, you would need to remember the previous value.

public boolean isConsecutive(ArrayList<Integer> list) {
    Iterator<Integer> it = list.iterator();
    if (!it.hasNext()) {
        return true;
    }

    int prev = it.next();
    while (it.hasNext()) {
        int curr = it.next();
        if (prev + 1 != curr /* mismatch */ || prev + 1 < prev /* overflow */) {
            return false;
        }
        prev = curr;
    }
    return true;
}