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.
--
Steven
_______________________________________________
Tutor maillist - Tutor@python.org
To unsubscribe or change subscription options:
http://mail.python.org/mailman/listinfo/tutor
