4
\$\begingroup\$

I made a word search searcher that searches whether a word search contains a given word. My code is super messy I know, way too many try blocks, nested loops etc. Maybe I've just used a bad approach. I'd love some tips on things like this. The program assumes the copied word search is a perfect rectangle or square.

charsInRow = input("Amount of characters in each row:\n")
charsInCol = input("Amount of characters in each column:\n")
allChar = input("Paste all the characters in:\n")


def Main():
    PopulateArray()
    FindWords()


def PopulateArray():
    global allChar
    allChar.lower()
    allChar = list("".join(allChar.split()))
    return allChar


def IsValid():
    global charsInRow
    global charsInCol
    try:
        charsInRow = int(charsInRow)
        charsInCol = int(charsInCol)
    except ValueError:
        return False
    return True


def FindWords():
    wordToFind = input("What word would you like to search?")
    wordToFind.lower()
    wordSearched = ""
    x = 0
    for n in range(0, len(allChar) - 1):
        if allChar[n] == wordToFind[0]:
            while x <= len(wordToFind):
                for i in range(0, len(wordToFind) * charsInRow, charsInRow):
                    try:
                        if allChar[n + i] == wordToFind[x]:
                            wordSearched = wordSearched + allChar[n + i]
                            n = n + i
                            break
                    except IndexError:
                        pass

                    try:
                        if allChar[n - i] == wordToFind[x]:
                            wordSearched = wordSearched + allChar[n-i]
                            n = n-i
                            break
                    except IndexError:
                        pass

                    try:
                        if allChar[(n + i) + 1] == wordToFind[x]:
                            wordSearched = wordSearched + allChar[(n+i) + 1]
                            n = (n+i) + 1
                            break
                    except IndexError:
                        pass

                    try:
                        if allChar[(n - i) + 1] == wordToFind[x]:
                            wordSearched = wordSearched + allChar[(n - i) + 1]
                            n = (n - i) + 1
                            break
                    except IndexError:
                        pass

                    try:
                        if allChar[(n - i) - 1] == wordToFind[x]:
                            wordSearched = wordSearched + allChar[(n - i) - 1]
                            n = (n - i) - 1
                            break
                    except IndexError:
                        pass

                    try:
                        if allChar[(n + i) - 1] == wordToFind[x]:
                            wordSearched = wordSearched + allChar[(n + i) - 1]
                            n = (n + i) - 1
                            break
                    except IndexError:
                        pass

                    try:
                        if allChar[(n + 1)] == wordToFind[x]:
                            wordSearched = wordSearched + allChar[n + 1]
                            n = n + 1
                            break
                    except IndexError:
                        pass

                    try:
                        if allChar[(n - 1)] == wordToFind[x]:
                            wordSearched = wordSearched + allChar[n - 1]
                            n = n - 1
                            break
                    except IndexError:
                        pass
                x += 1

    if wordToFind == wordSearched:
        print(True)
    else:
        print(False)

    print(wordSearched)


if IsValid():
        if len(allChar) == int(charsInRow) * int(charsInCol):
            Main()
        else:
            print("Not correct amount of characters, please try again")
else:
    print("Please input numerical values for amount of characters.")
input()
\$\endgroup\$
4
  • \$\begingroup\$ what is a "word search"? \$\endgroup\$
    – Hugo G
    Commented Feb 13, 2021 at 10:32
  • 2
    \$\begingroup\$ It's probably a good idea to show some sample input, to help those who don't understand the description. \$\endgroup\$ Commented Feb 13, 2021 at 12:22
  • 1
    \$\begingroup\$ @Hubert, I believe it's a form of puzzle where words are to be found in a grid of letters. All the words form straight lines horizontally, vertically or diagonally (in either direction). \$\endgroup\$ Commented Feb 13, 2021 at 12:24
  • \$\begingroup\$ If you run the code it works fine, you input in all the letters in the word search and give the amount in the rows and columns. Then it will ask for you to input a word to tell you whether or not it is on there. \$\endgroup\$
    – madine816
    Commented Feb 13, 2021 at 21:56

2 Answers 2

4
\$\begingroup\$

Naming

For your names to be PEP8-complicant, both your method names and variable names should adopt lower_snake_case, i.e.

chars_in_row
all_char
populate_array
find_words

Side-effects

IsValid is troubled. It does not do what it says on the tin - not only does it test validity of two variables, it actually converts them in-place. In addition, it improperly uses globals.

A better name for this would be try_parse, perhaps accepting a single string, not touching any globals, and returning an Optional[int] that is None if invalid.

Repeated tries

For your blocks that look like this:

                try:
                    if allChar[n + i] == wordToFind[x]:
                        wordSearched = wordSearched + allChar[n + i]
                        n = n + i
                        break
                except IndexError:
                    pass

there are two major issues. First, you should be looping through a list of indices starting with n + i, n - i, etc. so that you don't have to repeat those blocks; for example

indices = (
    n + i,
    n - i,
    # ...
)
for index in indices:
    # ...

Also, the fact that you're silencing IndexError suggests that your indexing is just wrong, and you need to re-examine your indexing bounds.

\$\endgroup\$
2
  • \$\begingroup\$ Not really sure what is meant by looping through a list of indices, how would that look? \$\endgroup\$
    – madine816
    Commented Feb 13, 2021 at 22:02
  • \$\begingroup\$ Edited; shown is a tuple instead of a list but same idea \$\endgroup\$
    – Reinderien
    Commented Feb 14, 2021 at 17:35
1
\$\begingroup\$

Computers should work for people, not the other way around. You don't need to force a user to tell you both the number of rows and columns; one dimension will suffice. Also, a user's command line terminal has many nice features such as the ability to recall previously entered commands so you don't have to retype things over and over. Your Python program lacks those conveniences. That means you can improve your user's experience by avoiding input() at all costs and instead receiving user input the standard, history-proven way: via command-line arguments and options. Python has a built-in library that does a decent job handling such arguments for you.

Global variables are almost never needed (other than constants, function definitions, and class definitions). That's because functions can pass arguments and return values among themselves.

Those two suggestions give us the following program structure. This is just a sketch. The implementation details for input validation, word searching, and user-friendly results printing are left to you.

import argparse
import sys

def main(args):
    # Get user arguments.
    opts = parse_args(args)

    # Validate user inputs.
    ...

    # Compute any need information, such as the number
    # of columns and other data you might want to assist with
    # searching (more on that below).
    cols = len(opts.characters) // opts.rows

    # Perform the word search.
    searches = find_words(opts.characters, opts.rows, opts.cols, opts.words)

    # Report to user. Enhance as needed.
    print(searches)

def parse_args(args):
    ap = argparse.ArgumentParser()
    ap.add_argument('characters')
    ap.add_argument('words', nargs = '*')
    ap.add_argument('--rows', type = int)
    return ap.parse_args(args)

def find_words(characters, rows, cols, words):
    # Do the searching, collecting results in some type
    # of data structure or object.
    searches = {
        w : w in characters
        for w in words
    }

    # Return that information to main().
    # Do not print() here. Keep algorithm and presentation separate.
    return searches

if __name__ == '__main__':
    main(sys.argv[1:])

As you noted, your current approach to perform the searches is very complex and repetitive. This makes it error prone, difficult to reason about, and awkward to change in the future. It also has some bugs: for example, in my attempted usage it failed to find words on a reverse diagonal. There's not much point in reviewing the code in its current state. You need a better plan.

There are many possible ways to simplify the situation. One suggestion is to stop trying to write word-searching logic and instead consider this question How could you reorganize the data to make word searching trivially easy? Here's one idea:

# Given this:
characters = 'catsdogseels'

# Create this:
grid = [
    # The rows.
    'cats',
    'dogs',
    'eels',
    'cde',
    # The columns.
    'aoe',
    'tgl',
    'sss',
    # The diagonals.
    'c',
    'da',
    'eot',
    'egs',
    'ls',
    's',
],

The rows are easy to create using Python's ability to slice into lists in various ways. The columns can be created by transposing the rows. You can search the internet for how to write a transpose() function in a few lines of code.

The diagonals are tricker, but here's one method:

# Start with the rows.
'cats'
'dogs'
'eels'

# Shift and pad them.
'cats  '
' dogs '
'  eels'

# Then transpose() those values and strip off the spaces.
\$\endgroup\$
4
  • \$\begingroup\$ one dimension will suffice. - OP states that the input may either be square or rectangular, so if you infer one dimension, you're unable to validate that the input has the correct length for the second. Maybe this is fine, but it's worth pointing out. \$\endgroup\$
    – Reinderien
    Commented Feb 15, 2021 at 13:42
  • \$\begingroup\$ avoiding input() at all costs is, I'm afraid, unable to justify. There are many reasons one might prefer stdin (which can be trivially redirected from the command line anyway) - including wanting to avoid pollution of the history buffer, or the need to enclose input in quotes, escaping edge cases, etc. Encourage the right tool for the job. In this case perhaps the dimensions could be optionally provided as arguments but it would be annoying to need the same for the bulk input. \$\endgroup\$
    – Reinderien
    Commented Feb 15, 2021 at 13:49
  • \$\begingroup\$ By "unable to justify" I mean "difficult to justify" - my grammar glands clearly aren't awake yet \$\endgroup\$
    – Reinderien
    Commented Feb 15, 2021 at 14:04
  • \$\begingroup\$ @Reinderien Perhaps I misunderstood what the OP is doing or overlooked something, but if you tell me there are 5 rows and give me 20 characters, 4 columns seems a safe inference. And your input() examples are mostly edge cases, in my experience. The vast majority of command-line programs don't need it. Consider the set of Unix tools: user interactivity is a tiny fraction of all usages. I groan every time I see Python educational materials encouraging new programmers to write so many CLI programs that revolve around tedious Q&A-style interactivity. It's misguided and teaches bad habits. \$\endgroup\$
    – FMc
    Commented Feb 15, 2021 at 18:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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