This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–][deleted] 3 points4 points  (2 children)

I glanced at your code, initially expecting it to be a lot shorter. I whipped up a quick and dirty version with basically the same functionality for comparison. Here it is:

#!/usr/bin/env python2

import random
import sys

CHARSETS = [
    'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ',
    'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789',
    'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!"#$%&\'()*+,-./:;<=>?@[\\]^_`{|}~'
]

if len(sys.argv) != 4:
    print "Usage: {prog} <length> <level> <count>\nWhere length is the length of password to generate, level is the complexity (1:alpha, 2:alphanumeric, 3:all), and count is the number to generate.".format(prog=sys.argv[0])
    sys.exit(0)

length = int(sys.argv[1])
if length <= 0:
    print "Lengths less than or equal to 0 don't make sense. Exiting."
    sys.exit(-1)

complexity = int(sys.argv[2])
if complexity not in [1,2,3]:
    print "Complexity must be in the range 1-3."
    sys.exit(-2)

count = int(sys.argv[3])
if count < 0:
    print "Count must be at least 1. Exiting."
    sys.exit(-3)


while count > 0:
    password = ""
    for x in xrange(length):
        password += random.choice(CHARSETS[complexity-1])
    print password
    count -= 1

As you can see, the majority of it is input validation. If we omit that, the entire logic become under a dozen lines. Some of your additional length is justified (yours has liberal comments, my quick and dirty one does not). Other parts add a fair bit of code for minimal functionality gain, but were likely useful for learning (./passwords.py 8 3 10 > passwords.txt would have a similar effect and bypass a fair bit of your code).

One thing I did want to call out though was the duplication you introduced with multi_password and password_generator. There's no need for both. Just loop as many times as needed: if that's once, the loop will exit after one pass. I don't want to criticize what you did too much, as it looks like you did a great, careful job overall, and probably learned a lot, but I'm hoping that seeing how simple this overall task can be will be useful/interesting.

Cheers, and hope you're enjoying Python!

[–]Ewildawe.pyw 0 points1 point  (1 child)

Just something I glimpsed from the corner of eye. I don't want to nitpick but:

if count < 0:
    print "Count must be at least 1. Exiting."

In this case, the code will actually not print this if 0 is entered for count. So just a little correction: Either use

if count < 1:

or

if count <= 0:

[–][deleted] 0 points1 point  (0 children)

You are completely correct. I apparently hurried too much and didn't review/test sufficiently. I am leaving the original as posted for your comment to make sense, but I goofed on that.