4
\$\begingroup\$

I have code that reads from files of surnames, female and male names and randomly generates a name in the following format: lastname, male/female name.

The files are in the following format:

JAMES 3.318 4,840,833 1

I have the following working code:

def picks(filename):
    with open(filename) as f:
        lines = [line.strip('\n') for line in f]
        pick = randint(0, len(lines))
    return lines[pick].split()[0]


def random_name(number_of_names):
    nselect = ''
    for _ in range(1, number_of_names+1):
        g = randint(0,1)
        if g==1:
            nselect = picks('femalenames.txt')
        else:
            nselect = picks('malenames.txt')
        print(g)
        print( '{0:s}, {1:s}\n'.format(picks('surnames.txt'), nselect))


random_name(5)

The code generates the correct output but I feel the code could be trimmed a bit.

\$\endgroup\$
5
\$\begingroup\$

Your intuition was accurate! Here is a shortened version with (almost-)identical functionality:

from random import choice

def picks(filename):
    with open(filename) as f:
        lines = [line.strip() for line in f]
    return choice(lines).split()[0]    

def random_names(number_of_names):
    for _ in range(number_of_names):
        nselect = picks(choice(('femalenames.txt', 'malenames.txt')))
        print('{0:s}, {1:s}\n'.format(picks('surnames.txt'), nselect))

random_name(5)

Note the following:

  • str.strip removes whitespace (including newlines) by default, so you don't need to make the argument explicit;
  • random.choice is a neater way to select a random item from a list than generating and using an index;
  • random_names is a better name for the function, as you can specify a varying number of names;
  • You don't need to initialise nselect;
  • Why carefully adjust an index you aren't actually using by one? 0 is the default start for range, so we can just use number_of_names directly; and
  • Selecting directly from the two possible filenames makes the functionality more obvious (although you can no longer print(g), which was a bad name anyway).
\$\endgroup\$
  • \$\begingroup\$ Not worth writing up another answer, but I'd consider memoizing the picks function (weakly, if memory is a concern). Sufficiently large files or desired number of names will make this sluggish if we have to pick every time. \$\endgroup\$ – Dannnno Jul 28 '15 at 20:26
  • \$\begingroup\$ @jonrsharpe. why is the choice in double brackets? \$\endgroup\$ – Leon Surrao Jul 29 '15 at 2:26
  • 1
    \$\begingroup\$ @LeonSurrao one set for the call, one set for the tuple to choose from. \$\endgroup\$ – jonrsharpe Jul 29 '15 at 6:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.