[Tutor] [newb] ascii questions

Alan Gauld alan.gauld at blueyonder.co.uk
Sun Dec 14 19:42:05 EST 2003


I'm not sure what exactly you are trying to achieve but
nonetheless here are some comments and opinions...

> def process(val):
>     r = val + random.randint(n, amnt)
>     if r < 0:
>         r = r + 26
>     elif r > 26:
>         r = r - 26
>     return r

You use n and amnt in the function so you would be better
to pass n and amnt in as parameters rather than rely on
global variables.

> count = 0
> name = Uname = raw_input('What is your name? ')
> while 1:
>     #user # as an int and verifies its in range
>     amnt = int(raw_input('Number 1 to 26? '))
>     if amnt > 26:
>         print 'That number is out of range'
>         continue
>     else:
>         #negetive user #
>         n = -amnt
>         break

If the user types a "correct" value then n is set
to the negative. Even if amnt is itself negative?
Shouldn't the check for invalid also test for <=0?
That is:

if 0 >= amnt < 26:
    n = -amnt
    break
else:
    print...

If the user types zero as it is then process(val)
will return val when val is between 0 and 26, is that
correct behaviour?

> #processes the users name 5 times
> while count < 6:

Using a while count when you know you want to do it
5 times is both counter intuitive and less readable
- especially with count intialised many lines above.
Better to use

for count in range(5)

>     #assigns the values of names
>     name = Uname
>     xname = ''
>     #gets 1st characters value
>     for x in name:
>         name_x = ord(name[0])
>         #uppercase
>         if name_x >= 65 and name_x <= 90:
>             name_x = name_x - 65
>             x = process(name_x) + 65


Why write assembler in Python? Why not just compare
to 'A' etc? Or even the isupper and islower string methods.
It might be marginally slower but it's much more readable.

We could rewrite it like this:

for c in name:
   if c.isupper():
      name_x = ord(c) - 65
      x = process(name_x) + 65


>         #lowercase
>         elif name_x >= 97 and name_x <= 122:
>             name_x = name_x - 96
>             x = process(name_x) + 96

and
   elif c.islower():
       name_x = ord(c) - 96
       x = process(name_x) + 96

and shouldn't 96 be 97??

>         #if name[0] is a [space]
>         elif name_x == 32:
>             x = 32

and
    elif c = ' ':
       x = 32

>         name = name[0:]

This line does nothing at all! Or more accurately it
assigns name to a new copy of itself...

>         #adds the values character to the processed name
>         xname = xname + chr(x)

Now we repeat for as many characters are in name, but
always processing the first letter? Assuming you wanted
to process each letter in turn then you could have
done it more simply without doing the extraction and
reassignment. I'm not really sure whether you mean to
repeatedly process the first letter or to process each
letter in turn?

>     count = count + 1
>     print xname
>     if count == 6:
>         print
>         print 'Hit Enter for more'
>         print 'Or ctrl-c to quit'
>         raw_input()
>         count = 0

This test could be rewritten:

again = raw_input("Repeat [y/n]? ")
if again in 'nN': break
else : count = 0

Asking users to hit control keys etc is a bit unfriendly IMHO.

Of course this test is why you used the while lop and not a 'for'
as suggested above. But if you use a test as I've shown you can
use a more radable while, like

repeat = ''
while repeat not in 'nN':
    ...for loop here...
    repeat = raw_input("Repeat [y/n]? ")


> Im not sure where the problem is in here. When I enter a name
with all
> lower case and a lower number for amnt I get back the results I
am
> expecting.

I'm not sure what you get back at other times, but make your
code easier to read and it will be easier to debug too...

Alan g.




More information about the Tutor mailing list