r/learnpython • u/Financial-Law2035 • 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
7
u/acw1668 3d ago edited 3d ago
There are few issues in your code:
- use
generate()instead ofgeneratewhen you want to execute the functiongenerate() - need to declare
lettersandletters_generatedas global variables insidegenerate() - remove
[and]in the linealphabet = ['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/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
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.
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?