[Tutor] Some questions about my yen-USD.py

[Andrei]

Dick Moores <rdm <at> rcblue.com> writes:
> (1) Have I handled possible user-errors OK?

I've tested it a bit and it seems to be quite robust.

> (2) Is my roundingN() function OK? Is there a better way to write it? 
> Will the line
> 
>         n = round(float(n)*(10**rounding))/(10**rounding)

Using format strings is easier I think. "%.2f" % 2.34234 will give '2.34'.

> get me into trouble with the flakiness of float(n)? In testing I 
> didn't find any problems, but ..

Nah. Float accuracy is only a problem if you need around lots of significant
digits (16 or so).

> (4) I wanted to call closingMessage() in main(), but in some cases, 
> if it's in main() it gets 2 successive calls. I can't figure out why. 
> Would IDLE's debugger be of use here? (I've never used it before.) I 
> haven't been able to find up-to-date IDLE help.

I think the main loop could be improved.

1) Python idiom is to *not* always call main(), because that makes it impossible
to use functions from your module in a different program: importing the module
would start running the converter, while you might only be interested in reusing
getRate(). It's better to put the following code at the bottom instead:

if __name__ == "__main__": # if module is running standalone (not imported)
    main()

2) If the application is used extensively, you'll exceed the maximum recursion
depth, because what the application does is essentially this (if the user
chooses to continue):

  main calls again calls main calls again calls... ad infinitum

This is called recursion - main essentially calls itself. You can test what
happens by disabling the entire main loop up to "again()" as well as the
raw_input line in again(). Python will quickly give up with:

    File "yen.py", line 164, in again
      main()
    File "yen.py", line 184, in main
      again()
  RuntimeError: maximum recursion depth exceeded

It's better to have a main() which does something like this:
  - get the conversion rate and store it in some (global?) variable
    (instead of asking for it every time)
  - start a while True loop:
    -- get the rate , currency and amount
    -- calculate and print result
    -- ask the user whether to do it again and stop loop if answer is No

3) The function again() uses an internal variable called 'again'. This is not
good coding practice (confusing). 
3b) Note that this is a variable local to the again() function, so you cannot
use it in the main() anyway:
  
  again()
  if again:
      break

"if again" actually checks if there is something called 'again' with a
non-zero/non-empty/non-None/True value. Your function is such a thing, so the if
condition always evaluates as True. (Tip: try "print again" before the "if
again", you'll see something like "<function again at 0x00B04670>"). This is of
course related to recursion problem above.

Improved version would be:

def again():
    answer = raw_input('Another?')
    return answer in 'yY' # evaluates to True if the user pressed 'y' or 'Y'

def main():
    # stuff
    while True:
        # stuff
        if not again(): 
            closingMessage()
            return None

Notice that this code is also easier to read than the original, because it's a
literal translation of you intention: if the user doesn't want to run it again,
say goodbye and stop. 

4) Function names are important and should be chosen in such a way that they are
not confusing. Most of them are OK, but commas() is non-descripting and
printResult() is confusing - it doesn't just print, it actually calculates. A
third party interested in reusing your calculation routine wouldn't expect the
printResult to be the culprit.

5) Checks like "if currency in 'USD'"  work, but are a bit unstable. It works in
this case because Yen and USD have no letters in common, so input of 'y', 'e' or
'n' all lead to 'Yen', while 'u', 's' and 'd' go to 'USD'. Imagine you'd extend
your program to also handle 'AUD', 'EUR', 'CAD' and 'SEK'. The 'e' would now
lead to Yen instead of EUR.

6) I would trim down the the checks in checkAmount:
- there is no need to use "continue" in that loop, because the loop will
continue anyway. 'continue' is only useful if there is some code later in the
loop that you want to skip.
- separate checks for negative and zero values seem pointless. Either handle 0
as any other number (gives result 0), or put a single check on "amount <= 0" and
inform the user that a number larger than 0 is required. 
Here's a modified version:

  def getAmount(currency):
      while True:
          useramount = raw_input('Amount: ').lower().strip()
          if useramount in 'qx':
              return useramount
          try:
               amount = float(useramount)
          except ValueError:
               continue # we want to skip the code below
          if amount <= 0:
              print "Amount must be more than 0."
          else:
              return amount

Using multiple returns in a single function is sometimes frowned upon; but then
again, so are break and continue :).

Yours,

Andrei