Thank you all for the help. I really appreciated the suggestions. Some of the things you pointed out, I originally used, but started changing thing when it wasn't working. I got it to work, but if you could let me know if there is anything I should do to make this code more pythonesque that would be great.
def rot_text(self, s): ls = list(s) for position, char in enumerate(ls): if char.isupper() or char.islower(): test = 'M' if char.isupper() else 'm' if char <= test: ls[position] = chr(ord(char) + 13) else: ls[position] = chr(ord(char) - 13) return "".join(ls) On Tue, Mar 12, 2013 at 4:11 AM, Karim <kliat...@gmail.com> wrote: > > What can be said after this detailed lesson...Bravo! > > > On 12/03/2013 11:58, Steven D'Aprano wrote: > >> On 12/03/13 16:57, RJ Ewing wrote: >> >>> I am trying to implement rot 13 and am having troubles. My code is as >>> follows: >>> >> >> A few comments follow. >> >> >> def rot_text(self, s): >>> ls = [i for i in s] >>> for i in ls: >>> if i.isalpha(): >>> if i.isupper(): >>> if i <= 'M': >>> x = ls.index(i) >>> ls[ls.index(i)] = chr(ord(i) + 13) >>> >> >> >> This is unnecessarily verbose and complicated. >> >> First problem, you use the name "i" to represent a character. Generally >> you should avoid single character names, and prefer meaningful names that >> explain what the variable represents. If you must use a single character, >> then the conventional names include: >> >> x, y, z: numbers, particularly floats >> x: (rarely) an arbitrary value of no particular type >> i, j, k: integer loop variables (rare in Python) >> n, m: other integers >> s: string >> c: character >> >> Notice that "c for character" makes much more sense than "i for >> character". >> >> >> You can, and should, create a list from the string using the list() >> function, rather than a list comprehension that just walks over the >> characters: >> >> ls = [c for c in s] # No, too verbose. >> ls = list(s) # Much better. >> >> >> You then loop over the list, again using "i for character". You can >> combine all those multiple if statements into a single clause: >> >> for i in ls: >> if i.isalpha() and i.isupper() and i <= 'M': >> >> But wait! The call to isalpha() is not necessary, because if a character >> is *not* alphabetical, isupper() will always return False: >> >> py> '9'.isupper() >> False >> >> So we can throw out the call to isalpha(). >> >> Your code to rot13 the letter is, sadly, broken: >> >> x = ls.index(i) >> ls[ls.index(i)] = chr(ord(i) + 13) >> >> >> This might make more sense if I re-write it using more meaningful names: >> >> position = ls.index(char) # This variable never gets used :-( >> ls[ls.index(char)] = chr(ord(char) + 13) >> >> The bit with chr() and ord() is fine. But what you do with it is wrong. >> The problem is that index always returns the position of the *first* copy >> of the item. So if your string is: >> >> 'ABA' >> >> the first time around the loop, your character is 'A'. ls.index('A') will >> return 0, and the rot13ed version, 'N', will be shoved into position 0. The >> second time around the loop, the character is 'B' and the rot13ed version >> gets shoved into position 1. But the third time, the character is 'A' >> again, index() will return 0 again, and the rot13ed version gets shoved >> into position 0 instead of position 2, giving you: >> >> 'NOA' >> >> >> The best way to fix this is to ignore index, and instead count which >> character we're at each time: >> >> >> position = -1 >> for char in ls: >> position = position + 1 # Advance the count. >> ... blah blah blah >> ls[position] = chr(ord(char) + 13) >> >> >> But wait! Python has a function to do exactly this for us: >> >> >> for position, char in enumerate(ls): >> ... blah blah blah >> ls[position] = chr(ord(char) + 13) >> >> >> >> >> Have a go with that, and see how far you get. >> >> >> >> >> > ______________________________**_________________ > Tutor maillist - Tutor@python.org > To unsubscribe or change subscription options: > http://mail.python.org/**mailman/listinfo/tutor<http://mail.python.org/mailman/listinfo/tutor> >
_______________________________________________ Tutor maillist - Tutor@python.org To unsubscribe or change subscription options: http://mail.python.org/mailman/listinfo/tutor