10
\$\begingroup\$

This code works, but I'm curious what feedback you have about style and doing things in a more "pythonic" way.

def printUsage():
    print("Usage: goodproxy.py -f <proxyfile> -u <url> -t <timeout>")


def main(argv):    

    filename    = ""
    url         = ""
    timeout     = None

    try:

        # USE GETOPS FOR EASIER PARAMETER PROCESSING
        opts, args = getopt.getopt(argv, "f:u:t:",["file=", "url=", "timeout="])

    except getopt.GetoptError:

        printUsage()
        sys.exit(2)


    # CHECK FOR MISSING PARAMETERS
    # THE 'OPTS' OBJECT CONSISTS OF A LIST OF TUPLES LIKE (PARAM,VAL)
    # LIST COMPREHENSION COLLECTS FIRST ELEMENTS FROM EACH TUPLE INTO A NEW LIST 
    # THEN TESTS THE NEW LIST FOR THE REQUIRED PARAMS 

    if not any(f in [opt[0] for opt in opts] for f in ['-f','--file']):
        printUsage()
        print("Error: -f parameter missing")
        sys.exit(2)

    if not any(u in [opt[0] for opt in opts] for u in ['-u','--url']):
        printUsage()
        print("Error: -u parameter missing")
        sys.exit(2)

    if not any(t in [opt[0] for opt in opts] for t in ['-t','--timeout']):
        printUsage()
        print("Error: -t parameter missing")
        sys.exit(2)


    # CONFIGURE SETTINGS BASED ON THE PASSED IN PARAMETERS
    for opt, arg in opts:

        if opt in ('-f', '--file'):

            print("Using proxies in: " + arg)
            filename = arg

        elif opt in ('-u', '--url'):

            print("Using URL: " + arg)
            url = arg

        elif opt in ('-t', '--timeout'):

            print("Timeout: {0}s".format(arg))
            timeout = arg



if __name__ == "__main__":
    main(sys.argv[1:])

I also found PyLint to assist with improving styling and code smells.

\$\endgroup\$
  • \$\begingroup\$ The coolest way I've seen to handle command-line parameters in Python is to use the docopt module. \$\endgroup\$ – 200_success Jul 18 '15 at 3:17
  • 1
    \$\begingroup\$ Honestly I'd strongly consider going with argparse. \$\endgroup\$ – shuttle87 Jul 18 '15 at 4:03
  • 3
    \$\begingroup\$ Please don't write comments in ALL CAPS - it looks like you're shouting at us. \$\endgroup\$ – jacwah Jul 18 '15 at 8:20
6
\$\begingroup\$

First off, I'd suggest that you read PEP8, Python's official style guide, on how to properly style your Python code.


Rather than creating a function to print usage, I'd recommend doing something like this instead:

USAGE = "Usage: goodproxy.py -f <proxyfile> -u <url> -t <timeout>"

You can then do this:

print(USAGE)

You shouldn't align the equal sign, =, when declaring variables. This means that this:

filename    = ""
url         = ""
timeout     = None

Should be changed to this:

filename = ""
url = ""
timeout = None

Python also has a standard for naming.

  1. Variables/function arguments should be in snake_case, and constant variables should be in UPPER_SNAKE_CASE.
  2. Functions should be in snake_case as well.
  3. Classes should be in PascalCase.

Finally, if you plan to do more advanced stuff with command line arguments like this, I'd recommend looking into the argparse library, which comes as a default Python package.

| improve this answer | |
\$\endgroup\$
8
\$\begingroup\$

Using argparse, your script can be greatly simplified:

from argparse import ArgumentParser


def main():
    parser = ArgumentParser(description='TODO')
    parser.add_argument('-f', '--file', metavar='PROXYFILE', required=True)
    parser.add_argument('-u', '--url', required=True)
    parser.add_argument('-t', '--timeout', required=True)
    args = parser.parse_args()

    print("Using proxies in: " + args.file)
    print("Using URL: " + args.url)
    print("Timeout: {0}s".format(args.timeout))


if __name__ == "__main__":
    main()
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ For a greenfield project I think this is definitely the way to go. \$\endgroup\$ – shuttle87 Jul 18 '15 at 15:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.