Generate a dictionary of passwords

Question

I have a Python script that generates a dictionary of all possible 3 digit password combinations. It essentially works by just nesting for loops. This is fine, but say I wanted to change it so it could generate 4, 5, 6, ... digit combinations. This would require me to keep adding more nested loops and changing the print statement.

How can I make my current code more efficient and apply the DRY principle? This way, if I wanted to change it to generate 4-digit codes, I could simply change a single variable rather then add an entire for loop.

Current code:

f=open('New Text Document.txt','w')
def getCombos(digits, caps, numbers):
    if(caps == True):
        if(numbers == True):
            for x in range(33, 126):
               for y in range(33, 126):
                   for z in range(33, 126):
                       print(chr(x) + "" + chr(y) + "" + chr(z))
                       f.write(chr(x) + "" + chr(y) + "" + chr(z) + "\n")

getCombos(3, True, True) 

If I wanted to add a fourth digit:

f=open('New Text Document.txt','w')
def getCombos(digits, caps, numbers):
    if(caps == True):
        if(numbers == True):
            for x in range(33, 126):
               for y in range(33, 126):
                   for z in range(33, 126):
                       for m in range(33, 126):
                       print(chr(x) + "" + chr(y) + "" + chr(z) + "" + char(m))


getCombos(3, True, True)

Answer 1

Please take a look at itertools.product. It will simplify your code greatly.

def getCombos(digits, caps, numbers):
    # Build set of choices appropriately based on caps and numbers
    choices = list(range(33, 126))
    for vals in itertools.product(choices, repeat=digits):
        print ''.join(map(chr, vals))

getCombos(3, True, True)

Answer 2

Instead of building characters using integers and chr, you can rely on the pre-built ones in the string module:

import string

def getCombos(digits, caps, numbers):
    symbols = string.ascii_lowercase
    if caps:
        symbols += string.ascii_uppercase
    if numbers:
        symbols += string.digits

    # process symbols to do what you aim for

Answer 3

In addition to @SjoerdJobPostmus's super tip, you can also use recursion, for example:

def print_combos(digits, prefix):
    if digits == 0:
        print(prefix)
        return
    for x in range(10):
        print_combos(digits - 1, '{},{}'.format(prefix, x))

print_combos(4, '')

There are some other issues in your code.

Instead of if(caps == True): you can write simply if caps:

You opened a file for writing but you forgot to close it. It's recommended to use a context manager, like this:

def getCombos(digits, caps, numbers):
    if caps:
        if numbers:
            # ...

with open('New Text Document.txt','w') as f:
    getCombos(3, True, True) 

Answer 4

My immediate reaction would be to just count with normal integers, and convert each to a string before use. Not entirely sure how that would work out for performance, but it seems like it would at least be simple and avoid repetition.

As far as example code goes...well, my knowledge of Python isn't quite to the point that I consider it exemplary (and that's putting it nicely) so I hope you'll forgive me to writing something in C++ instead.

I'd write code something on this order:

std::string cvt(int in, int lower, int range) {
    std::string ret;
    while (in) {
        ret.push_back(in % range + lower);
        in /= range;
    }
    return ret;
}

void gen_combos(int digits) {
    int lower = 33;
    int upper = 126;
    int range = upper - lower;

    int max = std::pow(range, digits);

    for (int i = 0; i < max; ++i)
        std::cout << cvt(i, lower, range) << "\n";
}

Do note that I'm not particularly advocating C++ for this task--it's almost certain to be I/O bound with any reasonable language; the only advantage to C++ (for me) is that I know it better than I do Python.

Looking at your code itself, I think the style is open to improvement. Right now, you have 33 and 126 sprinkled throughout the code; I'd try to give them more meaningful names.

I'd also avoid doing comparisons like if (x == True) and instead just use if (x)¹.

If you're going to test for both x and y being true before you do something, I'd prefer to use a logical and instead of nesting the if statements like:

if (x):
    if (y):
        // ...

It seems to me this is really generating things more than it's getting those things, so I'd at least consider changing the name to gen_combos (as above) or generate_combos or something on that general order.

---

^{1. Your code doesn't seem to do anything unless both numbers and caps are both true. In the code above, I just skipped these checks, at least for the moment. I think the real intent is that they probably control what characters can appear in the result string, so (for example) if caps is false, it's probably not supposed to use any capital letters--but that's just a guess, so I left it alone for now.}

Answer 5

@SjoerdJobPostmus made a good answer, but it's not very fast. Well it's really slow.
@Cilyan in his comments said that map is slow, while this isn't strictly true, it's correct.
Currently rather than the algorithm being \$O(n)\$ to change the range to characters, you and Sjoerd have a speed of \$O(kn^k)\$. To get \$O(n)\$ is easy, change it at creation.
And so I would encourage the following:

def get_combos(digits, caps, numbers):
    choices = map(chr, range(33, 126))
    for vals in itertools.product(choices, repeat=digits):
        print ''.join(vals)

You should split creation and display into two functions, to follow the single responsibility principle. So you should change it to:

def get_combos(digits):
    return (''.join(vals) vals in itertools.product(map(chr, range(33, 126)), repeat=digits))

I think the comprehension here is ugly, and can be better expressed by map. I also think the ~35% increase in speed is also quite nice. And so I would change it to:

def get_combos(digits):
    return map(''.join, itertools.product(map(chr, range(33, 126)), repeat=digits))

And usage would now be:

print('\n'.join(get_combos(3)))