Plz comment on this code

Discussion in 'Python' started by fridge, Sep 19, 2010.

  1. fridge

    fridge Guest

    # bigdigits2.py

    import sys

    zero=["***",
    "* *",
    "***"]
    one=["***",
    " * ",
    "***"]
    digits=[zero,one,zero,one,zero,one,zero,one,zero,one]

    inputted_digit=sys.argv[1]
    column_max=len(inputted_digit)
    row_max=3

    r=0
    while r<3:
    line=""
    c=0
    while c<column_max:
    digit_i=int(inputted_digit[c])
    digit=digits[digit_i]
    line+=digit[r]
    line+=" "
    c+=1
    print(line)
    r+=1
     
    fridge, Sep 19, 2010
    #1
    1. Advertisements

  2. [snip code]

    It looks like Python. Did you have a specific question?

    P.S. Please don't send HTML to non-binary news groups, it's very
    annoying. And if you're sending it by email, your mail client appears to
    be broken, because it's just dumping the HTML code after the plain text.

    [and much more HTML junk]


    P.P.S. If you can't even be bothered to spell "Please" in full, you
    probably shouldn't expect many people to be bothered to read your code in
    detail and comment extensively.
     
    Steven D'Aprano, Sep 19, 2010
    #2
    1. Advertisements

  3. fridge

    Peter Otten Guest

    - Add a docstring at the beginning of the script explaining what it is meant
    to do.
    - Use four-space indent; add a space between names and operators.
    - Replace the while-loops with for-loops. You can iterate over numbers and
    characters:
    .... print(i)
    ....
    0
    1
    2.... print(c)
    ....
    a
    b
    c

    Peter
     
    Peter Otten, Sep 19, 2010
    #3
  4. fridge

    Alex Willmer Guest

    Your code works (assuming digits gets populated fully), but it's the
    absolute bare minimum that would.
    To be brutally honest it's:
    - unpythonic - you've not used the core features of Python at all,
    such as for loops over a sequence
    - poorly formatted - Please read the python style guide and follow it
    - not reusable - Your code can only be called from the command line,
    it should be usable as a module
    - not documented - There is no indication what this code does, other
    than mentally running it
    - Fragile - There is no error checking on user input

    There are other ways to write what you have more concisely (e.g. list
    comprehensions, iterators) but those can wait for another time. Here
    is a start at improving your code wrt to the above points:

    #!/usr/bin/env python3

    # bigdigits2.py

    ZERO = ["***", # NB Constants are by convention ALL_CAPS
    "* *",
    "***"]
    ONE = ["** ",
    " * ",
    "***"]

    # TODO Define and populate digits 2-9
    DIGITS = [ZERO, ONE, ZERO, ONE, ZERO, ONE, ZERO, ONE, ZERO, ONE]

    def big_digits(str_of_digits):
    """Return a list of lines representing the digits using 3x3 blocks
    of "*"
    """
    banner = [] # Accumulate results in this

    # Loop over the rows/lines of the result
    # TODO Replace hard coded block size with global constant or
    measured size
    for row in range(3):
    line_parts = []

    # Assemble the current line from the current row of each big
    digit
    for digit in str_of_digits:
    big_digit = DIGITS[int(digit)]
    line_parts.append(big_digit[row])

    # Create a string for the current row and add it to the result
    line = " ".join(line_parts)
    banner.append(line)

    return banner

    def usage():
    print("Usage: bigdigit.py <number>", file=sys.stderr)
    sys.exit(1)

    if __name__ == "__main__":
    import sys

    # Check that an argument was passed
    # NB This will ignore additional arguments
    if len(sys.argv) >= 2:
    input_string = sys.argv[1]
    else:
    usage()

    # Check that only digits were passed
    if not input_string.isnumeric():
    usage()

    # All is well, print the output
    for line in big_digits(input_string):
    print(line)

    Here are some suggested further improvements:
    - Map directly from a digit to it's big digit with a dictionary,
    rather than indexing into a list:
    BIG_DIGITS = {
    "1": ["** ",
    " * ",
    "***"],
    # ...
    }
    - Is input_string.isnumeric() the right test? Can you find a character
    it would not correctly flag as invalid input?
    - What if I wanted to use my own 4x4 big digits? Could the
    big_digits() function accept it as an argument?
     
    Alex Willmer, Sep 19, 2010
    #4
  5. In message
    SAYS_WHO?
     
    Lawrence D'Oliveiro, Sep 19, 2010
    #5
  6. digits = [zero, one] * 5
    Defined but never used.
    Too many intermediate variables only defined and used once. Do this all on
    one line.
     
    Lawrence D'Oliveiro, Sep 19, 2010
    #6
  7. fridge

    Alex Willmer Guest

    Says PEP 8:

    Constants

    Constants are usually declared on a module level and written in
    all
    capital letters with underscores separating words. Examples
    include
    MAX_OVERFLOW and TOTAL.

    -- http://www.python.org/dev/peps/pep-0008/
     
    Alex Willmer, Sep 19, 2010
    #7
  8. In message
    WHAT_IF_SOMETHING_IS_INITIALLY_CONSTANT,_BUT_LATER_BECOMES_A_CONFIG_VARIABLE,_OR_VICE_VERSA,_DOES_IT_NEED_TO_CHANGE_ITS_NAME?
     
    Lawrence D'Oliveiro, Sep 20, 2010
    #8
  9. If you want to be compliant with PEP 8, then yes.
     
    Steven D'Aprano, Sep 20, 2010
    #9
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.