Random name generation in ruby

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)

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.

StackExchange Code Review Q#141460, answer score: 7