patternpythonMinor
Generating a random string of characters and symbols
Viewed 0 times
randomandgeneratingcharactersstringsymbols
Problem
After coding this, I was wondering if there are any ways in which I can improve upon it. It generates a random string of characters and symbols the same length as what is entered by the user. It uses a separate list for each char list but I am sure it is using a very long-winded method!
```
import random, sys
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"]
lettersnum = ["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","0","1","2","3","4","5","6","7","8","9"]
letterssym = ["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","#","*", "£", "$", "+", "-", "."]
lettersnumsym = ["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","0","1","2","3","4","5","6","7","8","9","#", "*", "£", "$", "+", "-", "."]
def generator(length, num, sym, caps):
count = 0
password = ""
try:
test = int(length)
except ValueError:
error("LENGTH NOT VALID NUMBER")
if num.lower() == "yes" and sym.lower() == "yes":
merge = lettersnumsym
elif num.lower() == "yes":
merge = lettersnum
elif sym.lower() == "yes":
merge = letterssym
while count <= int(length):
password = password + merge[random.randint(0, len(merge) - 1)]
count = count + 1
if caps.lower() == "uppercase":
password = password.upper()
elif caps.lower() == "lowercase":
password = password.lower()
print("PASSWORD:",password)
def error(error):
print(error)
sys.exit(0)
running = True
while running == True:
length = input("How long do you want the password?")
numbers = input("Do You Want It to Include Numbers?")
symbols = input("Do You Want It To Include Symbols?")
capital = input("Do You Want It To be uppercase or lowercase??")
generator(length, numbers, symbols, capital)
```
import random, sys
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"]
lettersnum = ["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","0","1","2","3","4","5","6","7","8","9"]
letterssym = ["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","#","*", "£", "$", "+", "-", "."]
lettersnumsym = ["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","0","1","2","3","4","5","6","7","8","9","#", "*", "£", "$", "+", "-", "."]
def generator(length, num, sym, caps):
count = 0
password = ""
try:
test = int(length)
except ValueError:
error("LENGTH NOT VALID NUMBER")
if num.lower() == "yes" and sym.lower() == "yes":
merge = lettersnumsym
elif num.lower() == "yes":
merge = lettersnum
elif sym.lower() == "yes":
merge = letterssym
while count <= int(length):
password = password + merge[random.randint(0, len(merge) - 1)]
count = count + 1
if caps.lower() == "uppercase":
password = password.upper()
elif caps.lower() == "lowercase":
password = password.lower()
print("PASSWORD:",password)
def error(error):
print(error)
sys.exit(0)
running = True
while running == True:
length = input("How long do you want the password?")
numbers = input("Do You Want It to Include Numbers?")
symbols = input("Do You Want It To Include Symbols?")
capital = input("Do You Want It To be uppercase or lowercase??")
generator(length, numbers, symbols, capital)
Solution
Python already defines a number of strings of possible characters. See
I would use
Two more point about separation of concerns with
-
It should return the password instead of printing it. Again, this allows the function to be used when not directly interacting with a command prompt.
-
It should throw an exception instance of calling
You convert
string.ascii_lowercase and string.digits SourceI would use
True and False instead of "yes"/"uppercase" as arguments to generator(). This function might be used by code that does directly interact with a user, so passing a string would not make sense. Additionally, the restart prompt supports a number of positive responses that are not supported by this function. You should have one layer that controls interaction with the user and one that generates a password. This will make the function cleaner as well as an easier API to work with.Two more point about separation of concerns with
generator():-
It should return the password instead of printing it. Again, this allows the function to be used when not directly interacting with a command prompt.
-
It should throw an exception instance of calling
sys.exit(). exit() will stop the python process and not allow any more execution of code. Your code is written so that multiple passwords can be generated one after another. However, if the user accidentally enters an invalid character to the first question, the application stops running instead asking the user to input a correct value. Throwing an exception would have the same result if you don't change the rest of your code, but allows you to change the code that interacts with the user to handle this case without restarting the application.You convert
length to an integer repeatedly instead of storing the value. The first validation completely ignores the result and the while loop does the conversion every time it tests if it should continue looping. This can all be solved by having length be passed in as an integer and letting the user layer handle the conversion and error cases.Generator is already a well defined term with in Python that do something very different. The function should be renamed to generate_password() or something similar.Context
StackExchange Code Review Q#48417, answer score: 6
Revisions (0)
No revisions yet.