Search code examples
pythonpython-3.xpassword-generator

I have questions on how to understand why my password generator is not executing properly


I'm a beginner programmer, so please forgive me if I'm not noticing the obvious.

I have run into problems with my password generator. First off, the code is not executing properly when I run the script, it prints "Your password is: ", and nothing else, the password won't generate.

I've checked the code with breakpoints, and nothing seems out of the ordinary. I'm just needing help to solve why it won't generate.

Once again, please forgive me if I'm not noticing the 'obvious'.

Thanks! And my code is shown below:

import random

Characters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
Special_Characters = "~!@#$%^&*()_"
Numbers = "1234567890"
Chosen_Characters = []
Chosen_Special_Characters = []
Chosen_Numbers = []
Password = ""
Nums_amt = 0
S_Character_amt = 0
Character_amt = 0
Total_characters = Nums_amt + S_Character_amt + Character_amt

def Amount_of_Characters():
    num = random.randint(1, 9)
    return(num)

def Character_chooser(A_o_C, Char_amt, Char):
    global Chosen_Characters
    for i in range(A_o_C):
        Chosen_Characters += [random.choice(Char)]
        Char_amt += 1
    return(Chosen_Characters)

def Special_Character_chooser(A_o_C, S_Char_amt, S_Char):
    global Chosen_Special_Characters
    for i in range(A_o_C):
        Chosen_Special_Characters += [random.choice(S_Char)]
        S_Char_amt += 1
    return(Chosen_Special_Characters)

def Number_chooser(A_o_C, Num_amt, Num):
    global Chosen_Numbers
    for i in range(A_o_C):
        Chosen_Numbers += [random.choice(Num)]
        Num_amt += 1
    return(Chosen_Numbers)

def Assembler(A_o_C, C_c, S_C_c, N_c, Total_char, S_Char_amt, Char_amt, Num_amt, Pword):    
    one = random.shuffle(C_c)
    two = random.shuffle(S_C_c)
    three = random.shuffle(N_c)
    for i in range(Total_char):
        chooser = random.randint(1, 3)
        if i + 1 <= Char_amt:
            if chooser == 1:
                temp_num = random.randint(0, len(one))
                Pword += one[temp_num]
                two.pop(temp_num)
        if i + 1 <= S_Char_amt + Char_amt and i + 1 > Char_amt:
            if chooser == 1:
                temp_num = random.randint(0, len(two))
                Pword += two[temp_num]
                two.pop(temp_num)
        if i + 1 > S_Char_amt + Char_amt:
            if chooser == 1:
                temp_num = random.randint(0, len(three))
                Pword += three[temp_num]
                two.pop(temp_num)
    return(Pword)

A = Amount_of_Characters()
B = Character_chooser(Amount_of_Characters(), Character_amt, Characters)
C = Special_Character_chooser(Amount_of_Characters(), S_Character_amt, Special_Characters)
D = Number_chooser(Amount_of_Characters(), Nums_amt, Numbers)

print("Your password is: " + Assembler(A, B, C, D, Total_characters, S_Character_amt, Character_amt, Nums_amt, Password))

Solution

  • TL;DR: your code assumes that if you pass a variable into a function and change the value of the parameter, it changes the original. It doesn't. That and some other mistakes cause the program to fail.

    Good programming style is not just a matter of looking good, it's about making your code easier to read for other programmers as well as your future self. So please allow me to comment on a few issues with your code:

    • You named a function Amount_of_Characters, but you shouldn't used capital letters in function names in Python since those signify classes, instead name it amount_of_characters
    • You then assign the function to result A, which doesn't really do anything, but a variable name should also be lowercase, name it a
    • After doing this for four separate functions, you pass all of them into another function (again, capitals, but you get it), which has parameters named like A_o_C, which is entirely non-descriptive and very hard to keep track of when mixed in with C_c, S_C_c, etc.

    Don't make your code look arcane and hard to read - nobody likes it and neither will future you.

    After looking at your code, it would appear that Amount_of_Characters just returns a random integer between 1 and 9, Character_chooser generates a list of n random characters from some string, Special_Character_chooser does the exact same thing (except that they both modify another global). And Number_chooser does the same again.

    Functions that have global side-effects are almost always a design mistake. Instead of manipulating a global variable, just return what the function is supposed to produce.

    Your Assembler finally proceeds to shuffle the order of the randomly chosen sequences - but since they were already random, that's pointless. It seems to expect that variables like Char_amt have been modified by the previous functions, but in fact they are not declared as global, so they don't have the correct values. If they did, the Assembler seems to select a number of characters randomly from the shuffled random strings.

    So, in the end, all your script does is:

    • generate a string of length n
    • each character of the string is randomly chosen from one of three collections of characters (English letters, numbers and some special characters) with equal odds

    Therefore, this script does the exact same thing (once you get yours to work):

    import random
    from string import ascii_letters, digits
    
    
    def generate_pass(n):
        chars = ['~!@#$%^&*()_', ascii_letters, digits]
        return ''.join([
            random.choice(chars[random.randint(0, 2)]) for _ in range(n)
        ])
    
    
    print(generate_pass(10))
    

    Everything else in your script is just moving stuff around and naming it. And applying random functions to something repeatedly doesn't necessarily make it more random. If you think the random library in Python somehow isn't random enough, you can find better libraries, but for the purpose of generating passwords, that would be pointless.

    And by the way: this is assuming you actually want about equal measures of numbers, letters and special characters in your password, otherwise it could be even shorter:

    import random
    from string import ascii_letters, digits
    
    
    def generate_pass(n):
        chars = '~!@#$%^&*()_' + ascii_letters + digits
        return ''.join([random.choice(chars) for _ in range(n)])
    
    
    print(generate_pass(10))