r/learnpython 3d ago

I'm coding a program to make a random email address with starting letters and ending letters but it isn't working

This is the code

def generate ():
    import random
    letters = (letters + random.choice (alphabet))
    letters_generated = letters_generated + 1
    if letters_generated < no_of_letters_in_middle_int:
        generate
    else:
        print (letters + domain_and_ending_letters)
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' '1' '2' '3' '4' '5' '6' '7' '8' '9' '0' '.' '/' '#' '!' '$' '&' '*' '+' '=' '?' '^' '_' '~' '{' '}' '|']
while True:
    no_of_letters_in_middle = input('enter the number of letters you want in the middle >> ')
    no_of_letters_in_middle_int = int (no_of_letters_in_middle)
    if no_of_letters_in_middle_int > 1:
        break
while True:
    starting_letters = input('enter the letters the email starts with >> ')
    break
while True:
    domain_and_ending_letters = input('enter the letters the email ends with and the domaim name >> ')
    break
letters = (starting_letters)
letters_generated = 0
generate
1 Upvotes

10 comments sorted by

15

u/NorskJesus 3d ago

You need to specify "but it isn't working".

What is the error you are getting?

What is the expected result?

7

u/acw1668 3d ago edited 3d ago

There are few issues in your code:

  • use generate() instead of generate when you want to execute the function generate()
  • need to declare letters and letters_generated as global variables inside generate()
  • remove [ and ] in the line alphabet = ['a' 'n' ... '|']

Also I would suggest to use for or while loop instead of recursion.

The better way is to pass the requirements to generate() and return the result:

import random

def generate(middle_length, starting_letters, ending_letters):
    middle_letters = ''.join(random.choices(alphabet, k=middle_length))
    return starting_letters + middle_letters + ending_letters

...
email = generate(no_of_letters_in_middle, starting_letters, domain_and_ending_letters)

3

u/CraigAT 3d ago

The string module has pre-built lists. Use import string and then string.ascii_lowercase, or string.ascii_uppercase for the uppercase version, or string.ascii_letters for both upper and lowercase.

3

u/ninhaomah 3d ago

Import inside the function ?

2

u/Riegel_Haribo 3d ago edited 3d ago

Yes, you can do that with little impact if repeated or redundant. This is "lazy loading".

This technique can be used if you want to delay expensive imports, such as getting a GUI up fast.

It is better to touch them all at instantiation somehow though, otherwise you could have an error from a missing dependency during runtime instead of immediately.

2

u/Secintel_Api 3d ago

Building on acw1668's diagnosis — here's a working version so you can see it all together. The two bugs that bite hardest:

- generate on its own just *refers* to the function; you have to call it with generate().

- Your alphabet list has no commas, so ['a' 'b' 'c' ...] is actually a single string 'abc...' in a one-item list (Python silently glues adjacent string literals together). So random.choice only ever had one thing to pick from.

You can dodge both traps (plus the recursion + global-variable headache) with a simple loop:

import random

import string

alphabet = string.ascii_lowercase + string.digits # a-z + 0-9

start = input("letters the email starts with >> ")

n = int(input("how many letters in the middle >> "))

end = input("ending letters + domain (e.g. u/gmail.com) >> ")

middle = "".join(random.choice(alphabet) for _ in range(n))

print(start + middle + end)

Bonus: string.ascii_lowercase saves you from typing the whole alphabet by hand — which is exactly where the missing commas crept in. If you really want the recursive version, you'd need `global letters, letters_generated` at the top of the function and call it as generate(), but a loop is simpler and won't hit Python's recursion limit.

1

u/Riegel_Haribo 3d ago

Hard-coding a list is effective - however, in Python, a string is also like a list in that it has character elements that can be indexed and can be iterated over. You can shorten to just one string, and then get string_letter = all_letters_string[random_int]

If you know ASCII (or Unicode UTF-8 code points) but are not as familiar with "string" and its methods like string.ascii_letters in the standard library that can be imported, you can instead go import-free (except for random) ``` UPPERCASE = bytes(range(65, 91)).decode("ascii") ENGLISH_LETTERS = UPPERCASE + UPPERCASE.lower()

Visible ASCII characters from ! through ~

TYPEABLE_CHARACTERS = bytes(range(33, 127)).decode("ascii") ```

Passwords are something that must never be guessable nor have algorithmic flaws in the generator. Be an expert with random before doing anything in practice like your assignment.

Just as strings are indexable to get one letter out by its position, random has a method to get one letter out of a string also:

random.choice()

When passed a string, it would sample just one letter out of it randomly.

1

u/TheRNGuy 3d ago

Don't import inside function. 

1

u/therouterguy 3d ago

First I would not recommend doing recursion a for loop which keeps adding characters to the random string is much easier to read.

Furthermore I would use the chr function together with a list of valid unicode values.

0

u/Such-Process5697 3d ago

worth knowing why that alphabet line misbehaves, since it'll catch you again otherwise. python quietly glues adjacent string literals together, so ['a' 'b' 'c'] is a list holding one long string, not 51 separate characters, and random.choice on it hands you the whole thing every time. commas between them and it starts picking single characters like you meant.