4
\$\begingroup\$

I finished CS50's bleep.py from pset6. I was hoping to get some feedback on my code (e.g., redundant code, bad practice, etc...).

Program function

  • load in a dictionary of banned words
  • accept a message from the user
  • if words in the message appear in the banned list, censor the word with *

    from cs50 import get_string
    from sys import argv, exit
    
    
    def main():
        # ensure correct usage
        if len(argv) != 2:
            print("Usage: python bleep.py dictionary")
            exit(1)
    
        # open dictionary of bad words
        name = argv[1] 
        text = open(name, "r", encoding="latin_1")
        if not text:
            print("Could not open {}.".format(name))
            exit(1)
    
        # store banned words in list
        bannedList = []
        for ban in text:
            ban = ban.replace('\n', '')
            bannedList.append(ban)
    
        # get user input to censor and split into words
        toCensor = input("What message would you like to censor? ")
        toCensor = toCensor.split()
    
        # for censored phrase
        final = []
    
        # check if word needs to be censored
        for word in toCensor:
            if word.lower() in bannedList:
                censored = word.replace(word, "*"*len(word))
                final.append(censored)
            else:
                final.append(word)
    
        print(' '.join(final))
    
    
    if __name__ == "__main__":
        main()
    

EDIT

I implemented the suggestions, and I like it a lot better now. It even handles commas, periods, and other punctuation now. Thanks again!

from cs50 import get_string
from sys import argv, exit
import re


def censor(text, banned_words):
    # for censored phrase
    final = []

    # check if word needs to be censored
    for word in text:
        if (word.lower()).strip(",.!?") in banned_words:
            censored = re.sub('[a-zA-Z]', '*', word)
            final.append(censored)
        else:
            final.append(word)

    #return censored phrase
    return ' '.join(final)

def main():
    # ensure correct usage
    if len(argv) != 2:
        print("Usage: python bleep.py dictionary")
        exit(1)

    # open dictionary of bad words
    name = argv[1] 
    with open(name, 'r', encoding="UTF-8") as f:
        lines = [line.rstrip() for line in f]

    # get user input to censor and split into words
    to_censor = input("What message would you like to censor? ")
    to_censor = to_censor.split()

    #censor and obtain new phrase
    censored = censor(to_censor, lines)
    print(censored)


if __name__ == "__main__":
    main()
\$\endgroup\$
3
\$\begingroup\$

Having a single main function that reads from os.stdin and writes the unrelated prompt to os.stdout is a bad architecture. In Python it is easy to create reusable modules, by using the following structure:

def censor(text, banned_words):
    """
    Returns a copy of the given text in which all words from
    banned_words are replaced with asterisks of the same length.
    """

    # TODO: insert some code here.


def main():
    # contains all the input/output stuff
    # TODO: insert some code here.


if __name__ == "__main__":
    main()

Programmed this way, other Python code can just import censor and then use the censoring code, no matter where the set of banned words comes from.

Having the code split into two separate functions also makes sense for understanding what the code does. If you're interested in how censoring works, look at the censor function. If you are interested in reading and writing to files, look in main.

Some other random remarks:

text = open(name, "r", encoding="latin_1")

Please don't use the latin_1 encoding if possible. It's been en vogue in the 1990, nowadays we prefer to use the full Unicode character set (including Arabic, Bengali, Chinese, Devanagari, Emoji, and many more) and UTF-8 to save Unicode text into byte-oriented files.

Most probably that file contains only ASCII characters anyway since CS50 takes place in the USA, where foreign languages and writing systems are often ignored.

text = open(name, "r", encoding="latin_1")
if not text:
    ...

This will not work since the open function either succeeds by returning a file object, or it fails and throws an exception. It never returns None. That's probably a leftover from your last exercise in the C programming language. But Python is very different from C.

Whenever you open a file, you must also close it when you're done with it. The simplest way is:

with open(name, 'r', encoding='UTF-8') as f:
    lines = f.readlines()

The with statement will automatically close the file at the end. It's not visible in the source code, you have to just know this.

toCensor = input("What message would you like to censor? ")

Be sure to only ever run this program in Python 3, since in Python 2 the input function behaves differently.

The name toCensor should rather be to_censor since in Python variable names and function names are written in snake_case instead of camelCase.

In the sentence This is BAD, BAD, BAD., assuming that BAD is a bad word, what would the expected output be? How can you change your program to do the expected thing? What should it do about BADABOOM?

\$\endgroup\$
  • \$\begingroup\$ That was incredibly helpful, and I really appreciate you taking the time to write this! Do you know of any Python books that you would highly recommend? \$\endgroup\$ – Snoop2001 May 1 at 16:51

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.