-1

I am beginning to learn Python and started experimenting with an example code block. I edited it a few times, and on the last edit that I did, I added an optional random password generator. Then I decided that it would make more sense to put the password generator into a separate document, so I copied the necessary code and made a new document. After editing it however, I cannot generate an even number of digits in the password.

Pastebin

Copy of Faulty Code (Pastebin)

import math
import random
alpha = ['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']
print('Would you like a random password suggestion generator', 'Yes or No')
permissionRandomGenerator = input().lower()
print('How long do you want your password?')
lengthRandomGenerator = int(input())
if permissionRandomGenerator == 'yes':
    def randInt():
        return math.floor(random.random()*10)
    def randChar():
        return alpha[math.floor(random.random()*27)]
    randPasswordList = []
    listInsert = 0
    def changeCase(f):
        g = round(random.random())
        if g == 0:
            return f.lower()
        elif g == 1:
            return f.upper()
    while listInsert < lengthRandomGenerator:
        randPasswordList.insert(listInsert, randInt())
        listInsert = listInsert + 1
        if listInsert >= lengthRandomGenerator:
            break
        randPasswordList.insert(listInsert, randChar())
        randPasswordList[listInsert] = changeCase(randPasswordList[listInsert])
        listInsert = listInsert + 1
        continue
    listInsert = 0
    printList = 0
    if lengthRandomGenerator <= 0:
        print('It has to be longer than that')
    elif lengthRandomGenerator >= 25:
        print('I can\'t generate a password that long')
    elif math.isnan(lengthRandomGenerator):
        print('error: not valid data type')
    else:
        while printList < (len(randPasswordList)-1):
            printItem = randPasswordList[printList]
            print(printItem)
            printList = printList + 1
    printList = 0
    randPasswordList = []
elif permissionRandomGenerator == 'no':
    print('Too bad...')
else:
    print('You had to answer Yes or No')
Ben Klemp
  • 3
  • 6
  • 2
    Here's a hint: name your identifiers *descriptively*. Naming variables/functions `a`, `b`, `c`, etc. is just asking for confusion and bugs. – MattDMo May 04 '16 at 00:01
  • 1
    I would suggest one additional improvement beyond what @MattDMo said to make it easier for people to help you. First read the documentation for python random. Everything you're doing with random.random() is much harder to read than built in operations you're performing. – Blake May 04 '16 at 00:19
  • @MattDMo I named them letters simply because they are used only in for, while, and if statements, and don't have much meaning by themselves. – Ben Klemp May 04 '16 at 02:33
  • 1
    In agreement with all the above comments, take a look at http://stackoverflow.com/questions/2257441/random-string-generation-with-upper-case-letters-and-digits-in-python first. You will get an idea to optimize your code. – user61092 May 04 '16 at 03:22
  • @Blake I'm not entirely certain what you are talking about. I did look at the site a little bit beforehand. The fastest and the easiest solution for me to understand was the random.random(), which I used in the same way I would use similar functions in languages like JavaScript. – Ben Klemp May 04 '16 at 03:37
  • @user61092 I am currently not worried about optimization, because I am trying to understand all of the basic principles through writing basic functions. I appreciate the suggestion, though. – Ben Klemp May 04 '16 at 03:40
  • @BenKlemp You're using the `d` variable, for example, on 13 lines in the above code. I've read your code a couple of times, and I *still* don't know what your thinking was, or what all the parts mean, and I'm no Python noob. `for`, `while`, and `if` statements make up nearly all of your code, so saying "I'm only using them there" is kind of silly. Yes, use `i` or something for this: `for i in range(10,51,5): print(i)`, but anything much more complex than that should have a descriptive name like `index` or `count` or something. Code is meant to be read by humans. Unnecessary brevity harms that. – MattDMo May 04 '16 at 15:00
  • @MattDMo I will attempt to rename some of the variables on my pastebin. I will probably put it into a new pastebin as well. – Ben Klemp May 04 '16 at 15:56
  • @MattDMo I just finished renaming the variables. Hopefully I renamed them all correctly. Here is the link to the new version: [Edited Code](http://pastebin.com/9k2Yn4gr) – Ben Klemp May 04 '16 at 16:19
  • @BenKlemp please use the [edit] function and put the updated code in your question. A question should be self-standing, meaning that it doesn't require someone to visit another site. – MattDMo May 04 '16 at 18:54
  • @MattDMo Did it right before/as you said. – Ben Klemp May 04 '16 at 18:56

3 Answers3

2

I refactored your program a bit, and got rid of a lot of unnecessary steps and inconsistencies. Here it is in full, then I'll explain each part:

import random
import string
import sys

possible_chars = string.ascii_letters + string.digits + string.punctuation

def nextchar(chars):
    return random.choice(chars)

yes_or_no = input("""
Would you like a random password suggestion generated?
Type Yes to continue: """).lower()

if yes_or_no == 'yes':
    try:
        pwd_len = int(input('How long do you want your password? '))
    except ValueError:
        sys.exit("You need to enter an integer. Please start the program over.")

    if 0 < pwd_len < 26:
        new_pwd = ""
        for _ in range(pwd_len):
            new_pwd += nextchar(possible_chars)
        print("Your new password is:\n" + new_pwd)

    else:
        print("I can only generate passwords between 1 and 25 characters long.")

else:
    print("Well then, why did you run me?")

Python is not just the syntax and builtin functions, it is also the standard library or stdlib. You're going to be working with the stdlib's modules all the time, so when you think you'll be using one, read the docs! You'll learn about the module, what its intended use is, some of its history and changes (such as in which version a certain function was added), and all of the classes, functions, and attributes contained therein. Make sure you read the whole thing (none of them are that long) and try to get at least a basic idea of what each thing does. That way, such as in this case, you'll be able to pick the best function for the job. One thing I like to do in my spare time is just pick a random module and read the docs, just to learn. They're generally fairly well written, and usually pretty inclusive. Get used to Monty Python references, they're everywhere.

import random
import string
import sys

Imports are first, and should almost always be only at the top. I like to put mine in alphabetical order, with the stdlib on top, then a blank line, then 3rd-party modules, including self-written ones next. Put a blank line or two after the imports as well. One thing to remember, that I mentioned in the comments: readability counts. Code is not only meant to be read by machines, but by people as well. Comment when necessary. Be generous with whitespace (also remember that whitespace is syntactically important in Python as well, so it forces you to indent properly) to separate related bits of code, functions, classes, blocks, etc. I highly recommend reading, rereading, and spending time pondering PEP-8, the Python style guide. Its recommendations aren't absolute, but many projects that enforce coding standards rely on it. Try to follow it as much as you can. If a line comes out to 83 characters, don't sweat it, but be aware of what you're doing.

The reason I made such a big deal out of reading the docs is the following few lines:

possible_chars = string.ascii_letters + string.digits + string.punctuation

def nextchar(chars):
    return random.choice(chars)

They get rid of about half of your code. string contains a bunch of predefined constants for working with strings. The three I chose should all be good valid password characters. If you're on a system that won't take punctuation marks, just remove it. Note that possible_chars is a string - like tuples, lists and dicts, strings are iterable, so you don't need to make a separate list of each individual possible character.

Next is the function - it replaces your randInt(), randChar(), and changeCase() functions, along with a bunch of your inline code, which was rather bizarre, to tell you the truth. I liked the method you came up with to decide if a letter was upper- or lower-case, but the rest of it was just way too much effort when you have random.choice() and the string constants from above.

yes_or_no = input("""
Would you like a random password suggestion generated?
Type Yes to continue: """).lower()

You may not have been aware, but you don't need to print() a description string before getting user input() - just pass the string as a single argument to input() and you'll get the same effect. I also used a triple-quoted """ (''' can also be used) string literal that differs from the more common single- ' and double-quoted " string literals in that any newlines or tabs contained within it don't need to be escaped. The take-home for now is that you can write several lines of text, and when you print() it, it will come out as several lines.

    try:
        pwd_len = int(input('How long do you want your password? '))
    except ValueError:
        sys.exit("You need to enter an integer. Please start the program over.")

I used a try/except block for the next part. If the user enters a non-integer up at the input prompt, the int() function will fail with a ValueError. I picked the simplest manner possible of dealing with it: if there's an error, print a message and quit. You can make it so that the program will re-ask for input if an error is raised, but I figured that was beyond the scope of this exercise.

    if 0 < pwd_len < 26:
        new_pwd = ""
        for _ in range(pwd_len):
            new_pwd += nextchar(possible_chars)
        print("Your new password is:\n" + new_pwd)

    else:
        print("I can only generate passwords between 1 and 25 characters long.")

Here is where all the action happens. Using an if/else block, we test the desired length of the password, and if it's between 1 and 25 (an arbitrary upper bound), we generate the password. This is done with a for loop and the range() function (read the docs for exactly how it works). You'll notice that I use a common Python idiom in the for loop: since I don't actually need the number generated by range(), I "throw it away" by using the underscore _ character in place of a variable. Finally, the else statement handles the alternative - either pwd_len is 0 or less, or 26 or greater.

else:
    print("Well then, why did you run me?")

We're at the end of the program! This else is paired with the if yes_or_no == 'yes': statement - the user entered something other than yes at the input prompt.

Hopefully this will help you understand a little bit more about how Python works and how to program efficiently using it. If you feel like you're spending a bit too much time implementing something that you think should be easier, you're probably right. One of Python's many advantages is its "batteries included" philosophy - there's a huge range of things you can do with the stdlib.

Community
  • 1
  • 1
MattDMo
  • 100,794
  • 21
  • 241
  • 231
  • As far as the characters, could you add a input/if group? `punc = input("""Would you like punctuation marks? """)` `if punc == 'yes':` `possible_chars = possible_chars + string.punctuation` Would that be syntactically correct? – Ben Klemp May 04 '16 at 21:23
  • @BenKlemp sure, that would work just fine. I just didn't do that in my program to keep things as simple as possible. There are all sorts of ways you can trick it out. – MattDMo May 04 '16 at 21:37
  • I will have to get used to the Python style of programming. The other language of this type that I know is JavaScript, which has a style more similar to what I was writing in. Thanks though, this was very helpful. – Ben Klemp May 04 '16 at 21:43
  • @BenKlemp yes, Python and JS are most certainly two different beasts altogether. Don't worry, the more programs you read, and the more you write, the more comfortable you'll get. JS is not "batteries included", although with ES6 it's getting a bit easier to do more complicated things more succinctly. Good luck with your studies! – MattDMo May 04 '16 at 21:48
  • `sys.exit("You need to enter an integer. Please start the program over.")` simply exits the program without saying anything. – Ben Klemp May 04 '16 at 21:50
  • @BenKlemp What version of Python are you using, and what operating system? It works fine for me, running the program from the command line using 3.5.1 under Linux... – MattDMo May 04 '16 at 21:55
  • I'm also using Python 3.5.1 but on a Windows 10 device. I ran the code using the Python IDLE. – Ben Klemp May 10 '16 at 03:25
0

I made some small edits, and my code seems to be working now. Here is the finished product (I put comments to show what the code does, and also to mark the edits.):

import math
import random                                 #Import necessary modules
alpha = ['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']  #List with alphabet
print('Would you like a random password suggestion generator', 'Yes or No')  #Prints the question for permission
permissionRandomGenerator = input().lower()   #Stores the answer of the above question in lower case
if permissionRandomGenerator == 'yes':        #Generates a password if the answer of the first question is 'yes'
print('How long do you want your password?')  #Asks for length
lengthRandomGenerator = int(input())          #Stores length as an integer
    def randInt():                            #Creates a random integer
        return math.floor(random.random()*10)
    def randChar():                          #Selects a random string from the list with the alphabet
        return alpha[math.floor(random.random()*27) - 1]
    randPasswordList = []                    #Creates a list to store the password
    listInsert = 0                           #Creates a list index variable
    def changeCase(f):                       #Defines a function to randomly change the case of letters before adding them to the list randPasswordList
        g = round(random.random())
        if g == 0:
            return f.lower()
        elif g == 1:
            return f.upper()
    while listInsert < lengthRandomGenerator + 1:  #Creates a random password and inserts it into randPasswordList  (I added `+ 1` here)
        randPasswordList.insert(listInsert, randInt())
        listInsert = listInsert + 1
        if listInsert >= lengthRandomGenerator:
            break
        randPasswordList.insert(listInsert, randChar())
        randPasswordList[listInsert] = changeCase(randPasswordList[listInsert])    #Calls the changeCase function whenever it inserts a letter
        listInsert = listInsert + 1
        continue
    listInsert = 0
    printList = 0
    if lengthRandomGenerator <= 0:           #If the length it 0 or less (for example, negatives) the password will not generate (I need to fix this a little bit.  Currently the code attempts to create a password beforehand)
        print('It has to be longer than that')
    elif lengthRandomGenerator >= 25:
        print('I can\'t generate a password that long')
    elif math.isnan(lengthRandomGenerator):  #Currently this doesn't do anything, it needs to be moved farther forward
        print('error: not valid data type')
    else:
        while printList < (len(randPasswordList)-1):    #Prints the list item by item
            printItem = randPasswordList[printList]
            print(printItem)
            printList = printList + 1
    printList = 0                             #Resets the variables
    randPasswordList = []
elif permissionRandomGenerator == 'no':
    print('Too bad...')
else:
    print('You had to answer Yes or No')

Note: I made this code purely to experiment and better learn basic aspects of Python. This code is not optimized, and is also not as random as I can (and will) make it.

P.S. Sorry if the comments are incomplete, I am still learning this language.

Ben Klemp
  • 3
  • 6
0

I don't know why you are doing over complicated for this simple problem, you can just use the constant provided by the string object, I would rather have the following programs to generate random password

import random, sys, string

def pgen(length=8):
    if length < 8:
        length = 8 
    keys = list(string.printable[:-6])
    random.shuffle(keys)
    return ''.join(keys)[:length]


if __name__ == '__main__':
    try:
        print( pgen(int(sys.argv[1])))
    except Exception as e:
        print("Provide length of password \n passwordgen.py <length_of_password>")

Outputs

magautam@nix1947:/tmp$ python passwordgen.py 12
HNLxi!{.qe=b

magautam@nix1947:/tmp$ python passwordgen.py 45
}w5u?+C=e[DfI.n'*1G(m{r0FH|UBKz/@kL>;Sh`tEW8-
shining
  • 1,049
  • 16
  • 31