treyhunner / names

Random name generator
MIT License
462 stars 107 forks source link

Magic number antipattern #18

Open IlyaSemenov opened 8 years ago

IlyaSemenov commented 8 years ago

In https://github.com/treyhunner/names/blob/master/names/__init__.py#L23:

selected = random.random() * 90

This is clearly an instance of magic number antipattern, the value of 90 must be documented.

sikrinick commented 7 years ago

selected = random.random() * 90 So selected cannot have values higher than 90. As far as I understood, this number represents summarized wage of names, for example, sum of second column in this file, while third represents cummulative wage: https://github.com/treyhunner/names/blob/master/names/dist.male.first But in this code, there is a consequent for-loop:

with open(filename) as name_file:
        for line in name_file:
            name, _, cummulative, _ = line.split()
            if float(cummulative) > selected:
                return name

This means, that it is impossible to get a name after name SHON

SHON 0.004 90.001 1208

All names after SHON are unreachable. My offer is to change hardcoded magic number 90 to a number, that represents cummulative wage of pre-last name. For example:

def get_name(filename):
    name_file = open(filename)
    #Using cummulative wage of second name from end
    prelast_cum_wage = name_file[len(name_file)-2][2]
    selected = random.random() * prelast_cum_wage
    for line in name_file:
        name, _, cummulative, _ = line.split()
        if float(cummulative) > selected:
            return name