Random name generation in ruby

Question

I created this program as an exercise for me to learn ruby. It is a random name generator with a set of rules defined to generate pretty decent and usually pronouncable names. Ruby isn't exactly my favourite language, but I needed to learn it. The names produced consist of both latin letters and a few non-latin symbols like å, ä and ö.

The code is fairly straight forward but I suspect there are a few things I do in the code that isn't considered good practise. It would be helpful in my path to learning ruby to get a code review on my program:

require 'unicode'
# Constants containing all consonants and vowels in the latin alphabet + some
# extra non-latin letters. The number after each letter represents how common
# a letter should be
CONS_LATIN = ['b']*100 + ['c']*100 + ['d']*100 + ['f']*100 + ['g']*100 + ['h']*100 + ['j']*100 + ['k']*100 + ['l']*100 + ['m']*100 + ['n']*100 + ['p']*100 + ['q']*85 + ['r']*100 + ['s']*100 + ['t']*100 + ['v']*100 + ['w']*50 + ['x']*75 + ['z']*50
VOWS_LATIN = ['a']*100 + ['e']*100 + ['i']*100 + ['o']*100 + ['u']*100 + ['y']*75
VOWS_EXTRA = ['ij']*75 + ['å']*100 + ['ä']*100 + ['ö']*100 + ['ø']*75 + ['æ']*60
# Banned combinations which are hard to pronounce or look weird
BANNED_COMBOS = [['g','j'],['f','k'],['b','k'],['q','p'],['w','q'],['q','g'],['x','x'],['q', 'q'],['d','b']]
def getRandomVowel
 # Only 10% chance to generate random "non-latin" vowel
 if rand() <= 0.1
 return VOWS_EXTRA.sample
 else
 return VOWS_LATIN.sample
 end
end
def getRandomVowelNoDuplicates(str:)
 # Generate a random vowel and if it a non-latin vowel
 # then we only use it if it has not been previously used in str
 vowel = getRandomVowel
 while VOWS_EXTRA.include? vowel and str.include? vowel
 vowel = getRandomVowel
 end
 return vowel
end
def getRandomConsonante
 return CONS_LATIN.sample
end
def getLastCharactersFromString(str:, numChars:)
 return Unicode::downcase (str[-numChars, numChars].to_s)
end
def isVowel(chr:)
 return ((VOWS_LATIN.include? (Unicode::downcase chr)) or
 (VOWS_EXTRA.include? (Unicode::downcase chr)))
end
def isConsonant(chr:)
 return (CONS_LATIN.include? (Unicode::downcase chr))
end
def generateLetter(currentName:)
 if currentName.empty?
 # We have a 60% chance to generate a vowel as the first letter
 if rand() <= 0.6
 return Unicode::upcase getRandomVowel
 else
 return Unicode::upcase getRandomConsonante
 end
 end
 if currentName.length < 2
 # Append random vowel or consonant in beginning to
 # prevent program from crashing if length of name is
 # less than 2
 if rand() <= 0.5
 chr = getRandomVowelNoDuplicates(str: currentName)
 else
 chr = getRandomConsonante
 end
 lastCharacters = chr + getLastCharactersFromString(str: currentName.join(""), numChars: 1)
 else
 lastCharacters = getLastCharactersFromString(str: currentName.join(""), numChars: 2)
 end
 # Apply rules
 #
 # 30% chance that there will be a double vowel
 # unless the last vowel is a non-latin vowel
 if isConsonant(chr: lastCharacters[0]) and isVowel(chr: lastCharacters[1]) 
 if rand() <= 0.3 and (VOWS_LATIN.include? lastCharacters[1])
 return lastCharacters[1]
 end
 end
 # No more than 2 consonants in a row
 if isConsonant(chr: lastCharacters[0]) and isConsonant(chr: lastCharacters[1])
 # Exception for 'chr' and 'sch'
 cons = getRandomConsonante
 if (lastCharacters == "ch" and cons == 'r') or
 (lastCharacters == "sc" and cons == 'h')
 return cons
 else
 return getRandomVowelNoDuplicates(str: currentName)
 end
 end
 # No more than 2 vowels in a row
 if isVowel(chr: lastCharacters[0]) and isVowel(chr: lastCharacters[1])
 return getRandomConsonante
 end
 # If no condition above is met we have a 40% chance to generate a vowel
 # and a 60% chance to generate a consonante
 if rand() <= 0.4
 return getRandomVowelNoDuplicates(str: currentName)
 else
 # Prevent weird combinations like gj, fk or bk.
 cons = getRandomConsonante
 for combo in BANNED_COMBOS
 while (lastCharacters[1] == combo[0] and cons == combo[1])
 cons = getRandomConsonante
 end
 end
 return cons
 end
end
def generateRandomName
 # Generate a number between 3 and 12
 # The reason we have 9 instead of 12
 # is because rand()*12 could give 12
 # and when we add 3, we would get 15
 # which would be greater than 12 (9+3 == 12)
 nameLength = (rand()*9+3).round
 # Create new list and initialize counter to 0
 name = []
 counter = 0
 # We loop nameLength times
 # So if nameLength equals 9 we loop
 # 9 times. We put the result of
 # generateLetter into our name list
 # at position counter. Counter will
 # be increased AFTER we put the result
 # into our name list
 currentName = ""
 nameLength.times do 
 name[counter] = generateLetter(currentName: currentName)
 currentName = name
 counter = counter + 1
 end
 # Convert the list of characters to a string using join("")
 return name.join("")
end
def askForNumber
 begin
 puts "Enter number of names to generate"
 num = gets.chomp
 end while num.to_i.to_s != num # check if num is a valid number
 return num.to_i
end
# Generate amount of names the user enters
num = askForNumber
num.times do
 puts generateRandomName
end

Here is an example output of the program (10 names generated):

Eznogio
Ool
Ekv
Elqepzea
Ypbovdioszom
Ghagy
Uggua
Owsieg
Ujgenmybho
Saomyehzod

Answer 1

Naming

Names are lowercase_with_underscores in Ruby, please avoid javaStyle for consistency with other Ruby code.

get is a precise Object Oriented principle, adding it in front of most of your functions name is confusing and unnecessary.

Repetition

def getRandomVowel
 # Only 10% chance to generate random "non-latin" vowel
 if rand() <= 0.1
 return VOWS_EXTRA.sample
 else
 return VOWS_LATIN.sample
 end
end

return and .sample are repeated twice, also 10% and 0.1 can be considered repetitions of each other and more in general the comment just states what is obvious from reading the function.

This is how I fixed these issues:

def random_vowel
 (rand() < 0.1 ? VOWS_EXTRA : VOWS_LATIN).sample
end

---

CONS_LATIN = ['b']*100 + ['c']*100 + ['d']*100 + ['f']*100 + ['g']*100 + ['h']*100 + ['j']*100 + ['k']*100 + ['l']*100 + ['m']*100 + ['n']*100 + ['p']*100 + ['q']*85 + ['r']*100 + ['s']*100 + ['t']*100 + ['v']*100 + ['w']*50 + ['x']*75 + ['z']*50
VOWS_LATIN = ['a']*100 + ['e']*100 + ['i']*100 + ['o']*100 + ['u']*100 + ['y']*75
VOWS_EXTRA = ['ij']*75 + ['å']*100 + ['ä']*100 + ['ö']*100 + ['ø']*75 + ['æ']*60

100 is everywhere. I suggest you initialize all values to 100 that is the default and then change the less common ones.

WEIGHTS = Hash.new 100 # 100 is default
[ ['w', 50], ['x', 75], ['q', 85] ].each{ |ch, w| WEIGHTS[ch] = w}
CONS_LATIN = ('a'..'z').reject{|x| "aeiouy".include?(x)}.map{|x| x*WEIGHTS[x]}.join

I also avoided typing the whole alphabet by hand as that leaves room for hard-to-find bugs of forgetting one letter.

Conceptual repetition

def getRandomVowelNoDuplicates(str:)
 # Generate a random vowel and if it a non-latin vowel
 # then we only use it if it has not been previously used in str
 vowel = getRandomVowel
 while VOWS_EXTRA.include? vowel and str.include? vowel
 vowel = getRandomVowel
 end
 return vowel
end

And:

def askForNumber
 begin
 puts "Enter number of names to generate"
 num = gets.chomp
 end while num.to_i.to_s != num # check if num is a valid number
 return num.to_i
end

You do an action until a condition becomes true in both cases.

To express this general concept we need to pass in functions as arguments.

def perform_action_until_condition(action, condition)
 # Both action and condition are functions
 # Action takes no arguments and returns type T
 # condition takes a type T argument and returns a bool
 # TO DO: Implementation
end

Here is an implementation in Python of this concept. In this example code you can see how the repetition of the two similar while loops has been avoided.

Simplifications

if currentName.empty?
 # We have a 60% chance to generate a vowel as the first letter
 if rand() <= 0.6
 return Unicode::upcase getRandomVowel
 else
 return Unicode::upcase getRandomConsonante
 end
end
if currentName.length < 2
 # Append random vowel or consonant in beginning to
 # prevent program from crashing if length of name is
 # less than 2
 if rand() <= 0.5
 chr = getRandomVowelNoDuplicates(str: currentName)
 else
 chr = getRandomConsonante
 end
 lastCharacters = chr + getLastCharactersFromString(str: currentName.join(""), numChars: 1)
else
 lastCharacters = getLastCharactersFromString(str: currentName.join(""), numChars: 2)
end

Becomes:

if currentName.empty?
 return Unicode::upcase (rand() < 0.6 ? random_vowel : random_consonant)
end
if currentName.length < 2
 chr = [random_unique_vowel(current_name), random_consonant ].sample
# ... same

---

def generate_random_name
 length = (3..12).to_a.sample
 name = []
 length.times {names << generate_letter(name)}
 return name
end

The random number between a min and a max is just a random choice between the range. You had un-used variables such as current_name, the counter was not needed as << adds in the last position and forcing named parameters from function callers is not common nor encouraged in Ruby.

---

The main problem of this code still remains the generateLetter (generate_letter`) function that remains huge and complicated, so the next step in re-factoring would be to divide it into helper functions.

The second problem is the lack of tests but given the randomness of this program, testing requires seeding the RNG and is less effective than on deterministic programs. Tests help to see if refactorings like mine do indeed keep the functionality as before or change it.

The best quality of this code is that you divided it into functions, one was too big (see issue one) but the division helped my understanding over a single equivalent "blob" (all top level) program enormously.

• Linus: Thanks for the detailed answer, I have a slight problem though. CONS_LATIN = ('a'..'z').reject{|x| "aeiouy".include?(x)}.map{|x| x*WEIGHTS[x]} generates an array of string and thus when I use CONS_LATIN.sample, I get a string and not a character.
• Caridorc: @Linus Adding .flatten should fix the problem.
• Linus: I had to use [x] aswell, so it becomes CONS_LATIN = ('a'..'z').reject{|x| "aeiouy".include?(x)}.map{|x| [x]*WEIGHTS[x]}.flatten
• Linus: How come I can't use a simple while loop for the random_unique_vowel and ask_for_number functions? I find it much easier to understand and I don't understand what perform_action_until_condition should do.
• Caridorc: @Linus the while loops are a special kind a loop, in the body the only action is assigning the result of an action to a variable. The same concept is being shared: here is an implementation in Python codepaste.net/9ey9hh

Answer 2

You should convert all these functions into methods of a class RandomNameGenerator. This makes them invisible for other parts of the code.

Instead of using the global rand function, create a new Random field in the class. For testing, you can initialize this field with a constant random seed.