1
\$\begingroup\$
map = [
    "......",
    "..##..",
    ".##.#.",
    "...###",
    "###.##",
]

x1 = 0
x2 = 0
obstacle= ()
obstacle = list(obstacle)
j=0

for x,y in enumerate(map):
    x1 = 1
    for i in range(0, len(y)):
        if len(y)- 1 == i and y[i] == "#" and y[i-1] == ".":
            x2 = i+1
            x1 = i+1
            obstacle.insert(j,(x1, x2, (x+1)))
            j+=1
        elif len(y)- 1 == i and y[i] == "#":
            x2 = i+1
            obstacle.insert(j,(x1, x2, (x+1)))
            j+=1
        elif len(y)- 1 == i and y[i] == ".":
            break   
        elif y[i] == "#" and y[i+1] == "." and y[i-1] == ".":
            x2 = i+1
            x1 = i+1
            obstacle.insert(j,(x1, x2, (x+1)))
            j+=1
        elif(y[i] == "#" and y[i-1] == "."):
            x1 = i+1
        elif(y[i] == "#" and y[i+1] == "."):
            x2 = i+1
            obstacle.insert(j,(x1, x2, (x+1)))
            j+=1
        
print(obstacle)

The program creates a list of obstacle locations, meaning where they begin and end and in which row. This program works as it is supposed to but i am wondering if this code can be more simplified and shorter.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Empty list

obstacle= ()
obstacle = list(obstacle)

This code creates an empty tuple, and assigns it to obstacle. Then, it takes that empty tuple, and converts it to an empty list, and assigns this new value to obstacle. A Python type checker will see the variable obstacle used as both a tuple and a list and complain that the type mutates.

If the goal is to initialize obstacle with an empty list, this can be achieve in one statement:

obstacle = []

Append

            obstacle.insert(j, …)
            j+=1

First, PEP-8 would require a space on either side of the += operator. But there is no need for the j variable at all. The code is insert values at the end of the obstacle list. This is simply an obstacle.append(…) operation!

Bug

        elif y[i] == "#" and y[i+1] == "." and y[i-1] == ".":
            ...
        elif(y[i] == "#" and y[i-1] == "."):
            ...

What happens when i == 0? y[i-1] will be y[-1] which actually looks at the last character in the row, which is most definitely not the character before y[0]! Does what happens make sense? The final result being correct for the wrong reasons is bad code.

Naming

You have x (an index), y (a string), i (an index into y), x1 and x2 (saved indices into y!?!?)

These names make no sense.

You refer to y as a “row” in your problem description. Let’s use that as a variable name:

for row_num, row in enumerate(map):
    ...

You never refer to x except as (x+1). Clearly you want 1-based indexing, so why not use the optional argument of enumerate to start at 1?

for row_num, row in enumerate(map, start=1):
    ...

Additionally, map is a Python function. While Python doesn’t object to you redefining its identified, it is frowned upon; you should use a different name, like obstacle_map to avoid confusion.

Special cases

You constantly repeat the len(y)- 1 == i test. You could refactor your code into nested if statements, to test this only once:

        if len(y)- 1 == i:
            if y[i] == "#" and y[i-1] == ".":
                x2 = i+1
                x1 = i+1
                obstacle.insert(j,(x1, x2, (x+1)))
                j+=1
            elif y[i] == "#":
                x2 = i+1
                obstacle.insert(j,(x1, x2, (x+1)))
                j+=1
            elif y[i] == ".":
                break   
        elif y[i] == "#":
            ...

But as noted earlier, you probably need y == 0 special case tests too. Let’s get rid of the special cases all together:

for row_num, row in enumerate(map, start=1):
    row = "." + row + "."
    for ch_idx, ch in enumerate(row):
        if ch == "#":
            ...

Since now the row is guaranteed to start with a period and end with a period, a # will never occur at the first or last character in the modified row, so ch_idx == 0 or ch_idx == len(row) - 1 is guaranteed to be False and never needs to be even checked!

As a bonus, adding the period to the start of the row automatically results in 1-based indices for the original row.

for row_num, row in enumerate(map, start=1):
    row = "." + row + "."
    for ch_idx, ch in enumerate(row):
        if ch == "#":
            if row[ch_idx - 1] == ".":
                start = ch_idx
            if row[ch_idx + 1] == ".":
                end = ch_idx
                obstacle.append((start, end, row_num))

str.find

Since we’re padding the start and end of each row with a period, an obstacle will always starts with ".#" and ends with "#.". This means we can use row.find("#") to locate the first obstacle’s start, and row.find("#.") to locate the first obstacle’s end. The str.find() has an optional argument which will allow the search to start further into the string, so we can use haystack.find(needle, start) to locate the first and subsequent obstacles start/end points:

for row_num, row in enumerate(map, start=1):
    row = "." + row + "."
    start = row.find("#")
    while start >= 0:
        end = row.find("#.", start)
        obstacle.append((start, end, row_num))
        start = row.find("#", end + 1)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.