[Solved] Python – Trying to make a Random String Generator

Question

Your code is mostly unsalvageable but I’ll go through it and explain all the issues I’ve encountered with it. You also didn’t indent your code properly in your question so I’ve made some assumptions on that front.

Addressing Code Issues

Alphabet generation

Alphabet = ["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"]`
Alphabet2 = ["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"]`

These could be more succinctly expressed as:

lowercase_letters = list(string.ascii_lowercase)
uppercase_letters = list(string.ascii_uppercase)

Iteration logic

Your current implementation will not iterate at all because you assign i = stringsize+1 and then create a while loop with the condition i < stringsize+1 – this will never be true when the condition is first evaluated.

The correct and Pythonic approach would be to use a for loop like this:

for i in range(stringsize):
    ...

String concatenation

Strings in Python are technically lists but it’s not really very pleasant to construct strings by appending individual characters to a list.

One approach would be to set StringGen = '' and then add characters to it using StringGen += c within the for loop. However, this isn’t efficient. I’ll provide a solution at the bottom of this post to demonstrate an implementation that does not involve concatenation within loops.

Misusing integers for conditional logic

The code:

MajorMin = randint(1,2)
if MajorMin == 1:
    ...
if MajorMin == 2:
    ...

Could be made far clearer using this equivalent logic:

use_uppercase_letter = random.choice([True, False])
if use_uppercase_letter:
    ...
else:
    ...

Alternative Implementations

Refined variation of your approach

Here’s a different implementation of randomstr that builds on the points here:

import string
import random


def randomstr(stringsize):
    lowercase_letters = list(string.ascii_lowercase)
    uppercase_letters = list(string.ascii_uppercase)

    def generate_letters(n):
        for i in range(n):
            use_uppercase_letter = random.choice([True, False])
            if use_uppercase_letter:
                yield random.choice(lowercase_letters)
            else:
                yield random.choice(uppercase_letters)

    return ''.join(c for c in generate_letters(stringsize))


print(randomstr(10))

My best crack at it

This is a much more concise implementation that I’ll offer in case you want it, but it deviates a lot from your original approach.

import string
import random


def randomstr(stringsize):
    letters = list(string.ascii_lowercase + string.ascii_uppercase)
    return ''.join(random.choice(letters) for _ in range(stringsize))


print(randomstr(10))

Example runs

These are the examples of the outputs you get with either of the implementations above.

MYXPupqiRG
ELNMPktrbe
ZnYBjlIxNQ

Accepted Answer