2

Here is the code that I have:

def reallyquit():
        from easygui import boolbox
        if __debug__: print ("realy quit?")
        answer = boolbox("Are you sure you want to Quit?")
        if answer == 1:
            sys.exit()
        return

columns = ['adr1','adr2','spam','spam', 'spam']

Street = choicebox("Street Address?", title, columns)
    while Street == None:
        reallyquit()
        Street = choicebox("Street Address?", title, columns)

Is there a more pythonic way to recursively ask a question if a user closes a box inadvertently? Mainly looking for tips on the last 4 lines, but feel free to critique the rest.

Anthon
  • 69,918
  • 32
  • 186
  • 246
Aaron
  • 23
  • 2
  • I don't see any recursion. – sberry Sep 06 '16 at 04:51
  • There are many non-pythonic aspects to your formatting, and using `== None:` is not pythonic either (use `is None:` instead). Please consider checking your source with `pep8` or `flake8`. Or directly run `yapf` on the source. – Anthon Sep 06 '16 at 04:54
  • 1
    Af few general pointers: 1. Don't import inside a function unless it is required (which is rare but does happen), keep your imports at the top of the file. 2. You don't need a "return" at the end of the function `reallyquit` since Python functions return None by default. 3. Indent (and verify formatting of) your code on SO. 4. What @Anthon said. – sberry Sep 06 '16 at 04:55
  • Also, you should not name your variables starting with an upper case letter. They could conflict with class names. – Soviut Sep 06 '16 at 04:58
  • And don't make one-line conditionals like `if __debug__: print("really quit?")`, that should be two separate lines. – sberry Sep 06 '16 at 04:59
  • 2
    Since there is nothing "wrong" with this code, it may be better to ask this question on [Code Review](http://codereview.stackexchange.com/) – chthonicdaemon Sep 06 '16 at 05:21

2 Answers2

2
import sys
from easygui import boolbox, choicebox

def reallyquit():
    "Ask if the user wants to quit. If so, exit."
    if __debug__: 
        print("really quit?")
    answer = boolbox("Are you sure you want to Quit?")
    if answer == 1:
        sys.exit()

columns = ['adr1', 'adr2', 'spam', 'spam']
street = None

# ask for address until given or user decides to quit
while street is None:
    street = choicebox("Street Address?", title, columns)
    if street is None:
        reallyquit()

In the main logic: Usually you would not test if a value is equal to None, since it is a singleton (only one in the system, and all occurrences of None are to the same logical None). Thus is None is a better test. I've also used a little more Pythonic code formatting (spaces after commas in a list, lowercase variable names), and a test order that doesn't duplicate the user interaction call.

An alternative form suggested by Eric would be something like:

while True:
    street = choicebox("Street Address?", title, columns)
    if street is not None:
        break
    reallyquit()

This has the virtue of not repeating the street is None test, but is a bit longer. Your preference for which option seems clearer and more logical may vary.

For the preparations: Imports are taken out of functions, and choicebox is imported where it was previously missing. The function is given a defining comment string. The one statement per line standard is enforced. And the unnecessary return removed.

This is still a fairly coarse program, but it's now "more Pythonic."

Not to be overwhelming, but the standard for Pythonic code appearance is PEP 8. There are tools such as pep8 you can install to check compliance. The deeper or "semantic" part of Pythonic code is sketched out in PEP 20, also known as "The Zen of Python." There is, sadly, no tool to judge compliance with those deeper principles. Experience is required.

Community
  • 1
  • 1
Jonathan Eunice
  • 21,653
  • 6
  • 75
  • 77
  • 1
    just notice your indentation of the last line `reallyquit()` is 3, should be 4 – Xiaojun Chen Sep 06 '16 at 05:04
  • Repeating the `street is None` condition here is not great - perhaps just do `while True`, and add a break to the if? That's the recommended way to emulate a `do while` statement in python – Eric Sep 06 '16 at 05:17
  • If line count really matters to you, `if street is not None: break` `reallyquit()` cuts one more off – Eric Sep 06 '16 at 05:34
  • @Eric It's really overall clarity that matters most. Line count is just one component. But I updated the alternate option with your `is not None` suggestion. It seems crisper. And that infinite-loop-plus-break should be able to put its best foot forward. – Jonathan Eunice Sep 06 '16 at 05:51
0

I think this is more idiomatic than using a sentinel variable:

while True:
    street = choicebox("Street Address?", title, columns)
    if street is None:
        reallyquit()
    else:
        break

Also, consider using the logging module for your debug printing, so instead of if __debug__: print(whatever) you just have logging.debug(whatever) and set the logging level.

chthonicdaemon
  • 19,180
  • 2
  • 52
  • 66