[Tutor] implementing rot 13 problems

[Karim]

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.
>
>
>
>