How to check that there is no `1` touching a border in a 2D-list of `0` and `1`-CodePudding

I need to consider a swimming pool "legitimate". For the given list, the function should return "illegitimate". However, my code returns "legitimate", even though I haven't done anything to the data.

This is the code that I tried and I was expecting/should return "illegitimate" before trying to modify the list.

pool = [[0, 0, 0, 0, 0],
        [0, 1, 1, 1, 0],
        [1, 1, 1, 0, 0],
        [0, 1, 0, 0, 0],
        [0, 1, 0, 0, 0]]

def is_legitimate_pool(pool):
    for r in range(len(pool)):
        for l in range(len(pool[r])):
            if pool[r][0] == 1 or pool[4][l] == 1:
                return str("illegitimate")
            elif pool[r][0] == 0 or pool[4][l] == 0:
                return str("legitimate")

print(is_legitimate_pool(pool))

CodePudding user response：

Solution

You could start off by checking if any element in the first and last sub-list is non-zero. Any non-zero integer i when passed to bool(i) will evaluate to True and only zero is "falsy" (see Truth Value Testing). This allows us to simply use the built-in any function for checking those two lists. If it returns True, at least one element is not zero.

Then we just iterate through the other sub-lists and check if their first or last element is falsy (i.e. zero). If at least one is not, we can immediately return. If we get to the end of the loop, that means the "pool is legitimate".

Code

LEGIT = "legitimate"
NOT_LEGIT = "illegitimate"

def is_legitimate_pool(pool: list[list[int]]) -> str:
    if any(pool[0]) or any(pool[-1]):
        return NOT_LEGIT
    for row in pool[1:-1]:
        if row[0] or row[-1]:
            return NOT_LEGIT
    return LEGIT

Test

test_pool1 = [
    [0, 0, 0, 0, 0],
    [0, 1, 1, 1, 0],
    [1, 1, 1, 0, 0],
    [0, 1, 0, 0, 0],
    [0, 1, 0, 0, 0],
]
test_pool2 = [
    [0, 0, 0, 0, 0],
    [0, 1, 1, 1, 0],
    [0, 1, 1, 0, 0],
    [0, 1, 0, 0, 0],
    [0, 1, 0, 0, 0],
]
test_pool3 = [
    [0, 0, 0, 0, 0],
    [0, 1, 1, 1, 0],
    [0, 1, 1, 0, 0],
    [0, 1, 0, 0, 0],
    [0, 0, 0, 0, 0],
]

print(is_legitimate_pool(test_pool1))  # illegitimate
print(is_legitimate_pool(test_pool2))  # illegitimate
print(is_legitimate_pool(test_pool3))  # legitimate

Caveat

The assumption is of course, that we are only interested in the "borders of the pool" being 0 and that an element can only ever be 0 or 1. If you actually need to explicitly check for border elements being 1s, we'd have to be a little more strict:

def is_legitimate_pool(pool: list[list[int]]) -> str:
    if any(element == 1 for element in pool[0]   pool[-1]):
        return NOT_LEGIT
    for row in pool[1:-1]:
        if row[0] == 1 or row[-1] == 1:
            return NOT_LEGIT
    return LEGIT

Errors in your code

There are a number of problems with your original function. One of them is that you must not return, before you have checked each sub-list. You need to check each of them, but you have a statement returning "legitimate", if your elif-condition holds, which would interrupt the loop as soon as just one row satisfies that condition.

The second problem is that you have your indices all messed up. The expression if pool[r][0] == 1 or pool[4][l] == 1 is the equivalent of saying "if the zero-th element in row r or the l-th element in row 4 equals 1". So you the second part is only ever checking row 4. You should check row r in both cases, but for the 0-th and 4-th element in that row being 1, so something like this: if pool[r][0] == 1 or pool[r][4] == 1.

Finally, you are not accounting for the fact that the very first row and the very last row must not contain any 1 at all. You'd have to check that at some point (preferably before starting to loop).

Optimizations

Fixing those problems would make the function work correctly, but it would still be sub-optimal because you would only be able to work on 5x5-lists of lists since you hard-coded the index 4 in a row to mean the "last" element. If you instead use index -1 it will refer to the last element no matter how long the list is.

For the sake of readability, you should avoid "index-juggling" as much as possible and instead leverage the fact that lists are iterable and can therefore be used in for-loops that yield each element one after the other. That way we can explicitly name and work on each sub-list/row in pool, making the code much clearer to the reader/yourself.

str("legitimate") is a no-op on the string literal "legitimate". You don't need the str function.

You should avoid shadowing global names in a local namespace. That means, if you have a global variable named pool, you should not also have a locally scoped variable pool in your function. Change one or the other so they have distinct names.