Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Magic number antipattern #18

Open
IlyaSemenov opened this issue Aug 17, 2016 · 1 comment
Open

Magic number antipattern #18

IlyaSemenov opened this issue Aug 17, 2016 · 1 comment

Comments

@IlyaSemenov
Copy link

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
Copy link

sikrinick commented Jan 16, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants