Code review for python password generator

The following code is a simple random password generator that spits out a randomized password based on some input flags.

import string
import random
import argparse

def gen_char(lower, upper, digit, special):

    _lower_letters = string.ascii_lowercase
    _upper_letters = string.ascii_uppercase
    _digits = string.digits
    _special = "!@#$%^&*()"

    _rand = random.SystemRandom()

    _case = ""

    if lower:
        _case += _lower_letters

    if upper:
        _case += _upper_letters

    if digit:
        _case += _digits

    if special:
        _case += _special

    if _case:
        return _rand.choice(_case)

    else:
        return _rand.choice(_lower_letters+_upper_letters+_digits+_special)

def main():

    """

    The following lines of code setup the command line flags that the
    program can accept.

    """

    parser = argparse.ArgumentParser()

    parser.add_argument("length", type=int, help="length of password")
    parser.add_argument("--lower", "-l", 
                       help="Generates passwords with lower case letters",
                       action="store_true")
    parser.add_argument("--upper", "-u",
                       help="Generates passwords with upper case letters",
                       action="store_true")
    parser.add_argument("--digit", "-d",
                       help="Generates passwords with digits",
                       action="store_true")
    parser.add_argument("--special", "-s",
                       help="Generates passwords with special characters",
                       action="store_true")

    args = parser.parse_args()

    """

    The following lines of code calls upon the gen_char() function to
    generate the password.

    """
    _pass = ""

    for x in range(args.length):

        _pass += gen_char(args.lower, args.upper, args.digit, args.special)

    print _pass

main()

The code is in working condition. I'm looking for tips in terms of coding styles, readability and perh

import string
import random
import argparse

def gen_char(lower, upper, digit, special):

    _lower_letters = string.ascii_lowercase
    _upper_letters = string.ascii_uppercase
    _digits = string.digits
    _special = "!@#$%^&*()"

Its against python convention to prefix local variables with _. Just don't do it. All of these categories would be better in a dictionary. That is create a global dict like:

CHARACTER_CATEGORIES = {
   'lower' : string.ascii_lowercase,
   'upper' : string.ascii_uppercase,
   'digits' : string.digits,
   'special' : "!@#$%^&*()"
}

Then take a list of categories as your parameters to this function instead.

_rand = random.SystemRandom()

You should create a random number generator once in your program, not once per random choice.

_case = ""

case? why case?

if lower:
        _case += _lower_letters

    if upper:
        _case += _upper_letters

    if digit:
        _case += _digits

    if special:
        _case += _special

    if _case:
        return _rand.choice(_case)

    else:
        return _rand.choice(_lower_letters+_upper_letters+_digits+_special)

this is really the wrong place to do this. the decision to include all categories if non are specifically selected is part of interpretating the user's input. So you should do it when you are parsing that, not here.

def main():

    """

    The following lines of code setup the command line flags that the
    program can accept.

    """

Not a very useful comment, because its pretty obvious that's whats happening here.

parser = argparse.ArgumentParser()

    parser.add_argument("length", type=int, help="length of password")
    parser.add_argument("--lower", "-l", 
                       help="Generates passwords with lower case letters",
                       action="store_true")
    parser.add_argument("--upper", "-u",
                       help="Generates passwords with upper case letters",
                       action="store_true")
    parser.add_argument("--digit", "-d",
                       help="Generates passwords with digits",
                       action="store_true")
    parser.add_argument("--special", "-s",
                       help="Generates passwords with special characters",
                       action="store_true")

    args = parser.parse_args()

    """

    The following lines of code calls upon the gen_char() function to
    generate the password.

    """
    _pass = ""

    for x in range(args.length):

        _pass += gen_char(args.lower, args.upper, args.digit, args.special)

Its not very efficient to do it this way as other have pointed out. But its also bad because adding strings in python is expensive, better to put all the letters in a list and join them.

print _pass

main()

Better to do

if __name__ == '__main__':
   main()

That way you could import this module elsewhere, and use its features.

StackExchange Code Review Q#17594, answer score: 6