[Tutor] Is my style OK in this elementary student exercise?

[Angus Rodgers]

>Date: Sat, 04 Jul 2009 13:26:12 +0100
>From: Angus Rodgers <angusr at bigfoot.com>
>Message-ID: <vrhu451924b136aea0ivadnlqc4h7uqavu at 4ax.com>
>
><http://python.pastebin.com/d7e245e0f>   (retention: 1 day)

This is just a quick(ish!) response to the feedback so far.

I realise, now, that I should have quoted what the exercise in the
book was asking for, to explain why my code is more elaborate than
the minimum that would be required to solve the problem at hand.

>From the book (2nd ed., p. 151):

 "[...] we recommend that the reader convert his or her code to
 functions that can be used in future exercises."

This requirement was uppermost in my mind.

I also remembered from previous brushes with programming (mostly
decades ago) that it is handy to have a little function that asks
a question and won't quit until the user gives a yes-or-no answer.

On the other hand, it would be overkill to try to write a program
that applies a full lexical, syntactic, and semantic analysis to
the text input by the user in an interactive console session.  In
the future I am quite likely to want to write something like that
(essentially reviving a student project from 1990, on which I got
a good enough mark to pass the course, but which suggested many
lines of further development - and whose implementation as my first
largish C++ program was simply abominable!), or else to use it
off-the-shelf from some Python library or other (suggestions?), 
but it would take months to write (perhaps weeks in Python, or
years in C++ ...), and therefore is out of the question here.

So I tried to take the middle course of writing small functions
that would be of use to me in future exercises (whether in this
book or elsewhere). The whole thing only took me an hour or so
to write (I didn't time the work, but it was certainly a matter
of minutes, rather than hours), so it wasn't overkill (I think!).

I did consider writing simpler code for input of strings as key
values, and, in a rewrite, I think I will provide such simpler
code, for this frequently-occurring default case (while still
allowing for general key types).

I also belatedly realised that I was stupidly duplicating code
in my get_dict() function, and I will rewrite this, as Daniel 
Woodhouse suggests.  get_dict() should make two calls to some
function with a name like get_val(), with a general type as a
parameter (perhaps defaulting to str - although the value type
parameter of get_dict() should, on reflection, have no default).

>------------------------------
>
>Date: Sat, 4 Jul 2009 18:50:48 +0300
>From: Daniel Woodhouse <wodemoneke at gmail.com>
>Message-ID:
>	<4c0a037d0907040850r7874313cn4abacc7db5957950 at mail.gmail.com>
>
>I had a look at your code, and refactored it a bit.  See my modifications at
>http://python.pastebin.com/m50acb143 You'll noticed I removed one of your
>functions completely (it was unnecessary) and the get_dict() function is not
>so heavily nested.

(See comments above.  I agree with the last bit, at least!)

>Some ideas:
>raw_input returns a string, so we can safely assume its a string.

It's definitely a good idea to treat this frequently-occurring
simple special case using simpler code (if only so that the user
doesn't have to enter those annoying quotation marks every time).

>Whether or
>not its a valid name for a student is a different matter, and if you wish to
>check such things you may wish to look at the regular expression module
>(re).

I'll certainly be investigating regular expressions (later), but
I think you'll agree that it would be overkill for this exercise.

>I wouldn't use eval as you did in the case, instead I did this to
>check the score is a valid int:
>try:
>    val = int(val)
>except ValueError:
>    print 'Invalid score'

I'll go halfway with you here.  I still want to keep the generality
of my code (for use in "future exercises"), but it shouldn't be
beyond my wit to avoid the overkill (and risk - see comment below)
of using eval(), and find out how to tell Python to evaluate a string
/only/ as a value of a known type (respectively, key_typ or val_typ).

I'll see if I can have a look at this after dinner, but I'll have
to do some reading of the book or the online documentation.  It's
probably just as simple as this:

ans = raw_input(prompt)
try:
    key = key_typ(ans)
except ValueError:
    (etc.)

except (ouch!) that I may still need to check for some more
general list of exceptions than just ValueError. (But I don't
want to start serious work on this just yet.)

>ValueError will catch invalid integers and we can ask the user to input
>again.  Generally we should only capture Exceptions that we are expecting,
>though there are exceptions to this (no pun intended)...

I'm sure you're right here.  I had an uneasy feeling that I was
doing something stupid and dangerous, which would not be a good
habit to get into.

>A lot of your code in get_dict() was repeated (basically the same operations
>to get the key and the value). You should try to avoid duplication if
>possible, either by putting the code in a seperate function, or just
>collecting everything at once as I have done.

Yes, I started to realise my mistake after posting. (Ain't it
always the way!)

>------------------------------
>
>Date: Sat, 4 Jul 2009 18:10:36 +0100
>From: Rich Lovely <roadierich at googlemail.com>
>Message-ID:
>	<f0b4202b0907041010q7aad7396y66a867a495d9b566 at mail.gmail.com>
>
>
>Lets consider how you handle your input.
>
>You get a string, from the user, then you pass it to eval, which runs
>it as if it was a python command.  Consider if they put in something
>like the following:
>__import__('os').system("rm -rf /")
>
>If you know linux, you will know this is a bad thing.  A similar idea
>on windows is __import__('os').system("format c: /q")

Even the vaguely recalled bit of Unix was enough to make me sweat!

>In short, unless you really know what you're doing, never use
>eval(raw_input(...)) for the same reasons you should never use
>input(...), for in effect, that is exactly what you are using.
>
>In fact, the python 3.0 docs recommend that form (using the new name
>for raw_input), in expert scripts allowing interactive code entry.

My response to this [but see the afterthought below!] is that I
definitely need to put into the documentation string something
like "*** THIS FUNCTION IS HIGHLY VULNERABLE TO A MALICIOUS USER
***", so that I will be strongly warned NEVER to incorporate it
into any program that might be used by anyone other than myself.

It might also be a good idea to include some kind of brief warning
in the user prompts. (This won't put off a malicious user, of course,
but it would help to keep me in mind of the potential danger.)

On the other hand, so long as I AM only executing the function
myself, I am no more at risk than I already am every single time
I type a command into a Python interpreter, of any description.
(A somewhat Existentialist thought, perhaps!  Virtual suicide
is always a possibility.) >->

Does that seem reasonable?  You've made me clearly aware of a
risk that I was only vaguely aware of before (I ruminated only
briefly as to whether any harm might come from entering general
Python expressions, but my main feeling about this facility was
that it would probably be useful - in some "future exercise"),
but isn't there a role for functions that one can use oneself,
but never ever distribute to the "general public"?  If so, are
the precautions I have suggested above sufficient?

(This question seems worth asking anyway, even though I have
already concluded, in my response to Daniel above, that I can
indeed avoid using eval() without losing anything essential.)
-- 
Angus Rodgers