10/15 Censor - Is my solution too complicated and if yes, why?


#1

So here's my question: I did solve 10/15 (phew!) but I think I did it wrong.
The next task I could solve in 6 lines, this one took me - many more.

So here is my question: did I do this too complicated? If yes, what is unneccessary?
Thanks for any help!

def censor(text,word):
    liste = list(text)
    for i in range(0 , len(liste)):
        if liste[i] == word[0] and \
        (i + len(word)) <= len(liste):
            n = 0
            while n < len(word):
                if liste[i + n] != word[n]:
                    break
                else:
                    n = n + 1
            if n == len(word):
                for c in range (0 , n):
                    liste[i + c] = "*"
    text = ''.join(liste)
    return text
censor("hey hey hey","hey")

#2

@specialsymbol

Nah, it's ok if it's complex if you come up with your own solution. It helps you understand what is going on better. At this point though you can start to get rid of the extraneous stuff and just have the meat and boats left.

Let's start with that.

First let's simplify what you have going on, it is quite easy once you get hang of it.

def censor(text, word):
    liste = []
    for i in text.split(' '):
        # This next line is not very logical so let's remove it.
        # if liste[i] == word[0] and (i + len(word)) <= len(liste):
        # Actually all the following code is not needed now
        # n = 0
        # while n < len(word):
        #     if liste[i + n] != word[n]:
        #         break
        #     else:
        #         n = n + 1
        # if n == len(word):
        #     for c in range (0 , n):
        #         liste[i + c] = "*"
        
        # This simple if/else is all that will be needed to find and replace words.
        if word.lower() not in i.lower():
            liste.append(i)
        else:
            liste.append('*'*len(word))
    # We needed a space here
    text = ' '.join(liste)
    return text

print(censor("hey hey hey", "hey"))

As you can tell I removed the whole body of your code and replaced it with just an if/else statement comparing the word to the current iteration we are looping over. If the current iteration is the word we append the appropriate items to the list else we append the word. Then we return a string method that joins the list with a ' ' in between each item.

Ok, now let's shorten it even further, we have a few tools at our disposal.

Python: Censor using trenary functions in conjunction with a generator expression

def censor(text, word):
    return ' '.join(('*'*len(word) if word.lower() in word_ else word) for word_ in text.split(' '))

print(censor("hey hey hey", "hey"))

As you can see we can get a lot of meat into one line, next let's use a python built in module. We are going to use a regular expression, which is a mini-language so you can use it in most other languages.

Regular Expression

from re import sub


def censor(text, word):
    return sub(word, ('*'*len(word)), text)

print(censor("hey hey hey", "hey"))

There you have it, all fancy and what not. If you need to do tons of replacements the regex module is run in c so it is fast as all get out, also you have the option to compile your regex so you can have it run even faster, do not try to optimize your code though, just make it readable to yourself.

One suggestion I am going to make to you is get into the habit of commenting your code, this will allow you to have a greater understanding of what is really going on in your code.

As always good luck and have fun!


#3

Ok, I didn't know you can search for an ordered set of consecutive elements in a string or even a list - I thought you had to check each element step by step. Thanks!


#4

although regular expressions are very powerfull I think a more pythonic resolution here would be to simply use the string replace method:

def censor(text, word):
  return text.replace(word,'*'*len(word))