5
\$\begingroup\$

I made a program to find words in a word grid. It will find words that are horizontal, vertical, positive sloped and vertical sloped, all in both forwards and backwards.

Here is my code:

with open('grid.txt', 'r') as f:
    lst = [[a for a in l.strip() if a != ' ' and a!= '\n'] for l in f.readlines() if l] # Convert file to a nested list, with all whitespaces removed
    lst2 = [a[::-1] for a in zip(*lst)] # Create another nested list that is the first nested list rotated 90 degrees clockwise
    
height = len(lst) # Height of grid
width = len(lst[0]) # Width of grid

while True:
    
    word = input("Input your word: ") # Word to find

    # Find horizontal word
    for l in lst:
        w = ''.join(l)
        if word in w or word in w[::-1]:
            print(w)
            print('Horizontal!')

    # Find vertical word
    for l in lst2:
        w = ''.join(l)
        if word in w or word in w[::-1]:
            print(w)
            print('Vertical!')

    # Find Negative slope
    for i in range(width):
        w = ''
        for j in range(height):
            try:
                w += lst[j][i]
                i += 1
            except:
                break
        if word in w or word in w[::-1]:
            print(w)
            print('Negative Slope!')

    for i in range(height):
        w = ''
        for j in range(width):
            try:
                w += lst[i][j]
                i += 1
            except:
                break
        if word in w or word in w[::-1]:
            print(w)
            print('Negative Slope!')

    # Find positive slope
    for i in range(height):
        w = ''
        for j in range(width):
            try:
                w += lst2[j][i]
                i += 1
            except:
                break
        if word in w or word in w[::-1]:
            print(w)
            print('Positive Slope!')
        
    for i in range(width):
        w = ''
        for j in range(height):
            try:
                w += lst2[i][j]
                i += 1
            except:
                break
        if word in w or word in w[::-1]:
            print(w)
            print('Positive Slope!')

Here is my grid.txt:

n x b z t a ê s e y m t w u e r r
y s j q l n o i l f k j c y h t a
q x n e z d l g v r x y x a p u a
b b k à a y e g q i h u x b q e u
m h z x r é q m x m e l t i b t q
i k i g c j c h w l w i ï t p u o
i s o w u w m a p z a b w g d e p
i p s g u y n n j i j f o f n w l
l i u l ô i e j b e o t s e b c j
y m f v f e v h f c p r d m y v n
f w h v d t z y i x t e e u v n e
e w x e h l x n q m s n p z e f b
e j l v r x ç t d b i a z v d h z
c i i n s s i b s t o q o d d k x
z o f c a w b j b l m w k f e i s

I feel like my code is unnecessarily long for a rather simple task.

Are there any ways to shorten and improve it?

\$\endgroup\$
  • \$\begingroup\$ Seems not to work. If I enter zxs from the lower-right, it does not output anything. \$\endgroup\$ – Reinderien Oct 18 at 2:24
  • 1
    \$\begingroup\$ It's because of encoding. After copying and pasting your example file, I had to specify UTF-8 for your open call to get this thing to work. \$\endgroup\$ – Reinderien Oct 18 at 2:28
  • 1
    \$\begingroup\$ The central slopes are printed twice, for example nsn from the top-left or zil from the bottom-left. \$\endgroup\$ – Marc Oct 18 at 2:34
  • \$\begingroup\$ Re. your most recent edit to describe strip() - technically this is an example of answer invalidation, because the feedback I gave is now partially incorrect given your new constraints. This is something that CodeReview tries to avoid, and as such - if we were to be strict - your most recent edit should be rolled back. Please keep this in mind for future questions: once you have answers, you generally shouldn't edit your question at least in substance. \$\endgroup\$ – Reinderien Oct 19 at 18:33
  • \$\begingroup\$ @Reinderien Oh! I'm very sorry! I'll roll it back. \$\endgroup\$ – user229550 Oct 19 at 23:56
5
\$\begingroup\$

Catch invalid input

When I was trying out your code, I entered Cntrl + Z which crashed the program. Using Try and Except can prevent this from happening

try:
    word = input("Enter your word:")
except Exception:
    print("Invalid input!")
  • Also, I'd keep a check of len(word) == 0 since an empty input is dangerous too

Reading grid.txt

with open('grid.txt', 'r') as f:
    lst = [[a for a in l.strip() if a != ' ' and a!= '\n'] for l in f.readlines() if l] 
    lst2 = [a[::-1] for a in zip(*lst)]

You can greatly simplify this part by using list.split()

with open('grid.txt', 'r') as f:
    lst = [line.split() for line in f.readlines()]
    lst2 = list( zip (*lst[::-1]) )

If you're worried about the '\n' at the end, the new lists using this method will not have it.

  • I'd suggest you keep the names of the lists as horizontal and vertical rather than lst and lst2 which don't mean much

Split work into functions

You should separate tasks into functions so that there isn't one huge block of code that does everything. Moreover, splitting work into functions makes it easy to maintain your code. What if your main game loop could look like this

def game_loop():
    while True:
        word = take_input()
        matches = find_matches(grid,word)

        if matches == None: print("No matches")
        else:
            for match in matches:
                print( "Match: " + match )

        if input("Do you want to play again? (y/n): ").lower() == 'n':
            break

An alternate approach

The reason your code isn't as efficient as the method I will just show you is that you repeatedly reverse the lists and use a lot of loops. There is a much better and cleaner way in which you need to go through the whole grid only once.

  • Read the .txt file into a list ( like we previously did for lst)
  • Start by going through each element in the 2-D list
  • If the element matches the first character of the entered word, check all 8 directions to complete the match

Implementaion

| improve this answer | |
\$\endgroup\$
3
\$\begingroup\$

There's a few things you need to avoid:

  • strip() when you really just need rstrip()
  • Forcing a caller of your program to loop through input; you should have functions to make this more divisible
  • Keeping a list of lists of characters. Just keep lists of strings.
  • Reversing the word on the inside of every loop; this should only be done once
  • Iterating blindly over a sequence until you hit IndexError
  • except: (you should catch the specific exception)

The following suggested implementation fixes your coordinate iteration so that you don't have to rely on exceptions.

def make_rot45(grid):
    m, n = len(grid[0]), len(grid)
    coords = (
        (
            (y + x, x) for x in range(max(0, -y), min(m, n-y))
        ) for y in range(1-m, n)
    )
    return [
        ''.join(grid[y][x] for y, x in coord_line)
        for coord_line in coords
    ]


def simple(word: str):
    with open('grid.txt', encoding='utf-8') as f:
        rot0 = tuple(
            line.replace(' ', '').rstrip('\n')
            for line in f
        )

    rot90 = tuple(''.join(col[::-1]) for col in zip(*rot0))
    rot45 = make_rot45(rot0)
    rot135 = make_rot45(rot90)
    word_rev = word[::-1]

    for grid, message in (
        (rot0, 'Horizontal!'),
        (rot90, 'Vertical!'),
        (rot45, 'Negative Slope!'),
        (rot135, 'Positive Slope!'),
    ):
        try:
            line = next(line for line in grid if word in line or word_rev in line)
            print(line)
            print(message)
        except StopIteration:
            pass


for test_word in (
    'def',
    'ehk',
    'gko',
    'sqo',
):
    print(f'Test word "{test_word}":')
    simple(test_word)
    print()

I ran this against the following file:

a b c
d e f
g h i
j k l
m n o
p q r
s t u

and it produced correct results.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Good points,+1, I missed them in my review. I wanted to clarify, is there any problem is using list.split() for reading the grid from the file? \$\endgroup\$ – Aryan Parekh Oct 18 at 16:56
  • \$\begingroup\$ Yes, there's at least two issues: it's not list.split(), but line.split(); and you shouldn't split() at all - simply replace. There's no need for an intermediate list representation. \$\endgroup\$ – Reinderien Oct 18 at 16:58
  • \$\begingroup\$ But doesn't .replace() return a string, compared to OP's implementation which uses a 2x2 list? \$\endgroup\$ – Aryan Parekh Oct 18 at 17:21
  • \$\begingroup\$ It's not a 2x2 list; it's a two-deep nested list. And yes, it's different from the OP's implementation intentionally. There's no sense in storing lists of lists of characters if all you're doing is string membership tests. \$\endgroup\$ – Reinderien Oct 18 at 17:24
  • \$\begingroup\$ Thanks. But for some reason, when I remove the try except part of your code, I get an IndexError. \$\endgroup\$ – user229550 Oct 19 at 17:43

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.