Generating simple and complex passwords

Question

I am pretty new to Python and I would like some tips and criticism on my formatting style/code and what can be better organized. I am a bit shaky on how classes work, but I think that it can be used here to make it more simple.

from tkinter import *
from tkinter import ttk
import random
#frame setup
main = Tk()
frame = ttk.Frame(main)
button_frame = ttk.Frame(frame)
text_frame = ttk.Frame(frame)
text = Text((text_frame), borderwidth=10, height=8, width=20, relief="sunken",)
def complex_password(): #complex password function
 password2 = []
 password2.append(random.choice((capital)))
 count = 0
 x = ""
 while count != 12:
 roll = random.randint(1,4)
 if roll == 1 or roll == 2:
 password2.append(random.choice((letters)))
 count +=1
 if roll == 3 or roll == 4:
 number = random.randint(0,9)
 number = str(number)
 password2.append(number)
 count +=1
 if count == 12:
 password2 = x.join(password2)
 text.insert(INSERT, password2)
 text.insert(INSERT, "\n")
def simple_password(): #simple password function
 y = ""
 password1 = []
 password1.append(random.choice((words)))
 password1.append(random.choice((words)))
 number = random.randint(1,99)
 num = str(number)
 password1.append(num)
 password1 = y.join(password1)
 text.insert(INSERT, password1)
 text.insert(INSERT, "\n")
def clear_text(): #clear txt box function
 text.delete(1.0, END)
#buttons
simple = ttk.Button(button_frame, text="Simple", command=simple_password)
complex = ttk.Button(button_frame, text="Complex", command=complex_password)
clear = ttk.Button(button_frame, text="Clear", command=clear_text)
#buttons grids
simple.grid(column=2, row=1)
complex.grid(column=1, row=1)
clear.grid(column=3, row=1)
text.grid()
#frame grids
frame.grid(column=1, row=2)
text_frame.grid(column=1, row=2)
button_frame.grid(column=1, row=1)
#misc settings
for child in frame.winfo_children(): child.grid_configure(padx=5, pady=10)
main.title("Password Gen")
main.resizable(width=FALSE, height=FALSE)
main.geometry("238x230")
words = ['Dog', 'Cat', 'Mouse', 'Fire', 'Ice', 'Basket', 'Tree', 'Tiger',
 'Lion', 'Flash','Super', 'Light', 'Zoom','Speed', 'Pants', 'Shirt',
 'Hat', 'Suit', 'Berry', 'Yogurt', 'Epic', 'Keyboard', 'Toe', 'Car',
 'Truck', 'Bike', 'Motor', 'Hammer', 'Pizza', 'Heart', 'Arm','Joint',
 'Saw', 'New', 'Carrots', 'Baby', 'Kiss', 'Backspace', 'Enter', 'Alt',
 'Print', "Down", 'Up', 'Question', 'Rain', 'Forest','Red', 'Orange',
 'Yellow', 'Green', 'Blue', 'Purple', 'Brown', 'Black', 'Indigo', 'Grey',
 'Shadow', 'Eye', 'Brick', 'Twig', 'Gangster', 'Thug', 'Chains', 'Gold',
 'Silver', 'Bronze', 'Platinum', 'Titanium', 'Exploding', 'Ladybug', 'Grass',
 'Monkey', 'Rhino', 'Comma', 'Hair', 'Shark', 'Fish']
letters = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm',
 'n','o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z']
capital = ['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M',
 'N','O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z']
main.mainloop() #end of GUI

Answer 1

I prefer

import tkinter as tk

and I can see that tk.Button, tk.INSERT, tk.FALSE are not my local variables/classes.

---

You can use string.ascii_lowercase instead of letters list, and string.ascii_uppercase instead of capital.

You don't have to convert string.ascii_lowercase and string.ascii_uppercase into list.

https://docs.python.org/3/library/string.html

---

password1 and password2 are local variables so you can use one name password in both functions.

---

for loop put in two lines - to make it more readable.

for child in frame.winfo_children():
 child.grid_configure(padx=5, pady=10)

---

In comments you can put one space after # - to make it more readable. You can put one space after every , .

---

You can use "" directly with join

"".join(password1)

---

You don't need ()

around text_frame in

tk.Text((text_frame), ...
# OK
tk.Text(text_frame, ...

around capitol in

choice((capital))
# OK
choice(capital)

around letters in

choice((letters))
# OK
choice(letters)

---

You can add empty lines in functions to separate some part of functions

---

In complex_password you can randint(1, 2) instead randint(1, 4) because it gives the same probability - 50% for letter and 50% for number.

In complex_password you can use for loop instead while and don't need variable count.

In complex_password you don't need if count == 12 because this part can be outside while/for loop.

---

You can organize code this way

import tkinter as tk
from tkinter import ttk
import random
import string
# === constants === (UPPER_CASE names)
 # empty
# === classes === (CamelCase names)
 # empty
# === functions === (lower_case names)
def complex_password(): #complex password function
 password = []
 password.append(random.choice(capital))
 for __ in range(12):
 roll = random.randint(1, 2)
 if roll == 1:
 password.append(random.choice(letters))
 else:
 number = random.randint(0, 9)
 number = str(number)
 password.append(number)
 password = "".join(password)
 text.insert(tk.INSERT, password)
 text.insert(tk.INSERT, "\n")
def simple_password(): #simple password function
 password = []
 password.append(random.choice(words))
 password.append(random.choice(words))
 number = random.randint(1, 99)
 password.append(str(number))
 password = "".join(password)
 text.insert(tk.INSERT, password)
 text.insert(tk.INSERT, "\n")
def clear_text(): #clear txt box function
 text.delete(1.0, tk.END)
# === main ===
# --- some variables ---
words = [
 'Dog', 'Cat', 'Mouse', 'Fire', 'Ice', 'Basket', 'Tree', 'Tiger',
 'Lion', 'Flash','Super', 'Light', 'Zoom','Speed', 'Pants', 'Shirt',
 'Hat', 'Suit', 'Berry', 'Yogurt', 'Epic', 'Keyboard', 'Toe', 'Car',
 'Truck', 'Bike', 'Motor', 'Hammer', 'Pizza', 'Heart', 'Arm','Joint',
 'Saw', 'New', 'Carrots', 'Baby', 'Kiss', 'Backspace', 'Enter', 'Alt',
 'Print', "Down", 'Up', 'Question', 'Rain', 'Forest','Red', 'Orange',
 'Yellow', 'Green', 'Blue', 'Purple', 'Brown', 'Black', 'Indigo', 'Grey',
 'Shadow', 'Eye', 'Brick', 'Twig', 'Gangster', 'Thug', 'Chains', 'Gold',
 'Silver', 'Bronze', 'Platinum', 'Titanium', 'Exploding', 'Ladybug', 'Grass',
 'Monkey', 'Rhino', 'Comma', 'Hair', 'Shark', 'Fish'
]
letters = string.ascii_lowercase
capital = string.ascii_uppercase
# --- init ---
main = tk.Tk()
main.title("Password Gen")
main.resizable(width=tk.FALSE, height=tk.FALSE)
main.geometry("238x230")
# --- objects ---
frame = ttk.Frame(main)
button_frame = ttk.Frame(frame)
text_frame = ttk.Frame(frame)
text = tk.Text(text_frame, borderwidth=10, height=8, width=20, relief="sunken")
# buttons
simple = ttk.Button(button_frame, text="Simple", command=simple_password)
complex = ttk.Button(button_frame, text="Complex", command=complex_password)
clear = ttk.Button(button_frame, text="Clear", command=clear_text)
# buttons grids
simple.grid(column=2, row=1)
complex.grid(column=1, row=1)
clear.grid(column=3, row=1)
text.grid()
# frame grids
frame.grid(column=1, row=2)
text_frame.grid(column=1, row=2)
button_frame.grid(column=1, row=1)
# misc settings
for child in frame.winfo_children():
 child.grid_configure(padx=5, pady=10)
# --- start the engine ---
main.mainloop()

• SuperBiasedMan: I edited your post to change - lines to being --- as that creates a light horizontal line, which (I think) looks nicer.
• furas: @SuperBiasedMan I used - because on StackOverflow it looks better than - - - (as for me) but here - - - looks better than on StackOverflow :)

Answer 2

Your code is quite confusing in some places. complex_password for example. Usually a function will return a value or run some process that modifies another value (eg a file on your computer or a class's attributes). It's unusual to see something like inserting the password result into a global object. It would be much nicer to return password2 and then call text.insert in the same place you call complex_password.

password = complex_password
text.insert(INSERT, password2)
text.insert(INSERT, "\n")

In your function you have a password2 but no password. If you were worried about duplicate names clashing then you needn't, password will just be a local value inside this function. It wont affect or be affected by other password values.

Instead of using a while loop with count, just use for _ in range(12):. That will loop twelve times with no need for a counter. Also x = "" is unnecessary, just call "".join.

This is how I'd rewrite the function:

def complex_password():
 password = [random.choice(capital)]
 for _ in range(12):
 roll = random.randint(1,2)
 if roll == 1:
 password.append(random.choice(letters))
 else:
 password.append(str(random.randint(0,9)))
 return ''.join(password)