Censor -- Constructive Criticism?


#1



https://www.codecademy.com/courses/python-intermediate-en-rCQKw/1/4?curriculum_id=4f89dab3d788890003000096#

Hello everyone! I've been working through this section's assignments, and found this one to be a little more difficult than those that came before. This was the first time I needed to grab a pen and paper to work out exactly how the logic needed to work before I could really make any progress.

Below is my (functioning) code. The exercise accepted a simpler version, but I wasn't happy with the way it handled near-matches for the censor word. This code doesn't use any special functions, really, only basic logical structures. I approached and tackled this problem all on my own, but I came here to see how I could improve my solution and perhaps even problem-solving abilities for the future.

I've seen different function solutions on this board that were a bit shorter than mine, which clocks in at 20 SLOC. I'm looking for tips to improve my efficiency and help me tackle problems like this more easily in the future.

And as an exercise, perhaps we can search for the shortest possible function that accomplishes the same task! :slight_smile:


def censor(text, word):
    stop = len(word)
    count = 0
    result = ""
    holder = ""
    for letter in text:
        if (letter == word[count]) & (count < stop):
            count += 1
            holder += letter
            if count == stop:
                result += '*' * stop
                count = 0
                holder = ""
        else:
            if count != 0:
                result += holder
                holder = ""
            result += letter
            count = 0
    return result

print censor("A man who does another mans dirty work is no man at all", "man")


#2

this is actually a good thing? So far, you have been coding. Now you start to program.

What kind of advise do you want? Speed optimazation? (strings are immutable in python, so you might want to consider to use lists)

how to reduce the number of lines of code?


#3

Consider making a list of words, as opposed to matching letter by letter.

stars = len(word) * "*"
censored = []
terms = text.lower().split();
print terms

['a', 'man', 'who', 'does', 'another', "man's", 'dirty', 'work', 'is', 'no', 'man', 'at', 'all']

Now iterate over the list and compare to word.

for term in terms:
    if term == word:
        censored.append(stars)
    else:
        censored.append(term)
return ' '.join(censored)

#4

Wow, thanks for the insight! I saw the split function introduced in the hint, but that was after I had already crafted my own solution. I hadn't imagined that it would produce a function so much more elegant!

I know this is sort of a big question, but do you have any other built-in Python functions (for strings or otherwise) that you find exceptionally useful? Again, I know that the language is quite extensive - so I just mean some of your personal favorites; something a student might commit to memory.


#5

You are right, of course! I enjoyed the challenge - that's what this discipline is all about. Any tips would be great from a more experienced programmer, but really I was looking for tips to make the function itself less unwieldy. (simplify the logic, learn other built-ins that I'll add to my regularly used toolbox) In this respect I quite like the code that mtf offered below.

I've come across the .replace() function, which I suspect would make this even simpler - but I haven't got the time to test it out yet as I'm about to go into my day job. Thanks again for your time and the speedy response.


#6

I've never really thought about that, and being a fledging Python user my usual first steps are procedural, like yours, which gets whittled down and refactored as I read the docs looking for useful tools that fit my immediate need.

I love what we can do with str.join(object).

>>> a =  '_'.join('abcdefg')
>>> a
'a_b_c_d_e_f_g'

It's not just for lists.

str.replace() is a very powerful tool.

>>> a.replace('_', '')
'abcdefg'
>>>

Then there is a convenience method for capitalizing a string...

>>> import string
>>> string.capwords(text)
"A Man Who Does Another Man's Dirty Work Is No Man At All"
>>>

We could go on, but you'll have fun doing this on your own.


#8

.replace() is not good enough in this case, if you then want to censor is, is of this also gets censored

i think @mtf covered everything how can use the built in .split() function to make live a lot easier. python has a wiki page with all built in functions


#9

I really like your approach - nice work.
The mentioned solution from mtf was the way i solved this task too - but there are a lot of other possibilities too:
One of them is:

def censor (text,word):
	while True:
		try:
			position = text.index(word)
		except ValueError:
			return text
		text = text[:position] + "*"*len(word) + text[position+len(word):]

i have to mention that this will replace occurrences within words too - like mentioned from stetim94


#10

Your approach is quite clever. However, as mentioned by @inter2k3, there are indeed many other ways to approach this problem. For instance, (please don't hate me for this), but here is my overly simple solution:

def censor(text, word):
    if word in text:
        return text.replace(word, (len(word)*"*"))
    else:
        return text

#11

replace is not good enough? you only want to censor whole words, if we use replace for:

censor("this is wack", "is")

you would get:

th** ** wack

while you want to get:

this ** wack

#12

Fair point. However, it works for the purposes of the exercise...


#13

i stopped caring about it works for the exercise a long time ago, instead i care my program/algorithm covers all cases and corner cases, design, elegance, readibility, which are far more important factors


#14

Hey, if there's anybody who gets wanting a program to work properly, it's me. I was just pulling your leg when I said that. Unfortunately, I hadn't noticed my program was functioning in that manner until you said something. Now that you have, I am quite humbled.


#15

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.