4
\$\begingroup\$

This is a basic name generator I just wrote and want some feedback. It basically chooses between a vowel or a consonant and appends it to a string. Note that it never puts two vowels or two consonants directly together, as it checks for that.

import random
vowels = ['a', 'e', 'i', 'o', 'u']
consonants = ['b', 'c', 'd', 'f', 'g', 'h', 'j', 'k', 'l', 'm', 'n', 'p', 'q', 'r', 's', 't', 'v', 'x', 'y', 'z']
def generate_name(length):
 if length <= 0:
 return False
 full_syl = ''
 for i in range(length):
 decision = random.choice(('consonant', 'vowel'))
 if full_syl[-1:].lower() in vowels:
 decision = 'consonant'
 if full_syl[-1:].lower() in consonants:
 decision = 'vowel'
 if decision == 'consonant':
 syl_choice = random.choice(consonants)
 else:
 syl_choice = random.choice(vowels)
 if full_syl == '':
 full_syl += syl_choice.upper()
 else:
 full_syl += syl_choice
 return full_syl
for i in range(100):
 length = random.randint(3, 10)
 print(generate_name(length))
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Mar 4, 2017 at 13:52
\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

Why does generate_name(0) return False? I would expect an empty string '', or maybe None.


Rather than check if full_syl == '': (not a great name, by the way; why not just name?) during the loop, I would build the whole name in lowercase then use str.title: return full_syl.title(). This removes both .upper and .lower from the forloop, simplifying the code.


Why pick every letter randomly? In this part:

decision = random.choice(('consonant', 'vowel'))
if full_syl[-1:].lower() in vowels:
 decision = 'consonant'
if full_syl[-1:].lower() in consonants:
 decision = 'vowel'

the first value of decision is ignored for every letter but the first. If you want strict alternation, you only need to randomly pick the type of the first letter. You could even use itertools.cycle to switch back and forth between vowels and consonants for each pick.


Rather than keep adding letters to a string, I would probably build a list or iterator of letters then str.join them. Otherwise you create many unnecessary string objects for the partial names, which are immediately discarded.

answered Mar 4, 2017 at 15:07
\$\endgroup\$
0

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.