Lottery number picker

Question

This is my first Python project. Let me know how I did, and where I can improve it.

# Random Lottery Number Pick Tool
# It has U.S. national lottos and two from Nebraska
# I'll add more to it later.
import random
# here is a giant body function I wonder if I could make this easier to deal with?
# it is basically my program's menu items.
def workerFunction():
 while True:
 try: x=input( '\n\n Select an option:\n\n'
 'Press 1+Enter for Pick Three\n'
 'Press 2+Enter for Pick Five\n'
 'Press 3+Enter for Powerball picks\n'
 'Press 4+Enter for Mega Millions picks\n'
 'Q+Enter to quit \n\n' )
 #This is my end of file logic that kills the program
 # Provided you pass Q + Enter
 except EOFError: pass
 if x.lower().startswith('q'):
 print("done")
 break
 # Otherwise you are passing something else that requires work from my factory!
 elif x == '1': print( '\n\n Nebraska Pick Three',
 factory.repeater( 3, factory.factoryLogic, [0, 9, 1] ) )
 elif x == '2': print( '\n\n Nebraska Pick Five',
 factory.factoryLogic( 1, 38, 5 ) )
 elif x == '3': print( '\n\n Powerball',
 factory.factoryLogic( 1, 55, 5 ),
 factory.factoryLogic( 1, 42, 1 ) )
 elif x == '4': print( '\n\n Mega Millions',
 factory.factoryLogic( 1, 75, 5 ),
 factory.factoryLogic( 1, 15, 1 ) )
# My factory class (self is set to something)...
# Not sure how to use it yet outside of the program I know it is like "this" in javascript.
class factory:
 # so I am defined self which looks like a struct. I assign it 0 (to shut up the syntax checker)
 def __init__( self ): self.a = 0
 # My logic module picks numbers based on a start and end position.
 # Not sure why I use +1 in the range function.
 # And then there are the number of interators I need.
 def factoryLogic( startPosi, endPosi, interateNumber ):
 a = random.sample( range( startPosi, endPosi+1 ), interateNumber )
 a.sort()
 return a
 # This is a repeater utility I made because pick three needs to be called three times.
 # The neat thing is that I am als passing a functions and args as a list and breaking them out.
 # I could make it more terse but why? It also returns an appended list. Neat!
 def repeater( times, f, functionArgs ):
 return_list = []
 for i in range( times ): return_list.append( f( functionArgs[0], functionArgs[1], functionArgs[2] ) )
 return return_list
# This name main holds my worker function. I call it workerFunction! Amazing, yes?
if __name__ == '__main__':
 workerFunction()

• jonrsharpe: Obligatory link to style guide: legacy.python.org/dev/peps/pep-0008
• jonrsharpe: Is this written for Python 2.x or 3.x?
• Frank Tudor: I think it would work on both? I don't think I used anything version specific.
• jonrsharpe: input and EOFError suggests you're not using raw_input for 2.x, which is bad form (and your class doesn't inherit from object, so will be old-style in 2.x).
• Frank Tudor: @jonrsharpe Oh I see...I'm going through your notes. Thanks for giving so much feedback.

Answer 1

The code is not compliant with the style guide. For example:

• x and a aren't descriptive variable names;
• It doesn't follow the naming convention (e.g. factory should be Factory and workerFunction should be worker_function);
• The inline comments should be replaced with docstrings;
• Extraneous whitespace (e.g. for i in range( times ): should be for i in range(times):); and
• Compound statements (e.g. except EOFError: pass should be on two lines).

---

The Pythonic replacement for lots of elif clauses is a dictionary. There are numerous ways to implement this, e.g.:

lotteries = {1: {'name': 'Nebraska Pick Three',
 'func': factory.repeater
 'args': (3, factory.factoryLogic, [0, 9, 1])},
 ...}

This simplifies the printout at the start:

print("Make a choice: ")
for key in sorted(lotteries):
 print("{0}. {1}".format(key, lotteries[key][name]))

and input validation:

if int(x) not in lotteries:

and the output:

lottery = lotteries[x]
print lottery['func'](*lottery['args'])

---

If you do want to stick with the current text block, Python does have multiline strings:

s = """Three quote marks
 makes a string
 multiline."""

For help on formatting, see Function to print command-line usage for a program.

---

The class implementation seems pointless - you have an __init__ that does very little (self.a is never referenced again), and two instance methods that don't need any class or instance attributes. If you want an OOP approach, I would be tempted by something like:

class Lottery:
 def __init__(self, rules):
 self.rules = rules
 def draw(self):
 ...

You can then put instances into the dictionary:

lotteries = {1: {'name': 'Nebraska Pick Three',
 'lottery': Lottery(...)},
 ...}

and the code gets even simpler:

lottery = lotteries[x]['lottery']
lottery.draw()

You will have to come up with a method for representing the different rules. I would suggest having a Boolean for when repeat draws are allowed, rather than a sample, instead of calling a separate method as you do now. Different lotteries could alternatively be subclasses, rather than instances, of Lottery.

---

Here's an example implementation (note: for Python 3.x):

import random
class Lottery:
 """Simulate a specified lottery."""
 def __init__(self, name, rules):
 """Define the lottery name and draw rules."""
 self.name = name
 self.rules = rules
 def draw(self):
 """Simulate a single draw of the lottery."""
 output = []
 for rule in self.rules:
 output.extend(self._draw_rule(rule))
 return output
 def _draw_rule(self, rule):
 """Helper function to draw a single rule."""
 if rule.get('repeats', False):
 output = [random.randint(rule.get('min', 1), rule['max'])
 for _ in range(rule.get('picks', 1))]
 else:
 output = random.sample(range(rule.get('min', 1), rule['max']+1),
 rule.get('picks', 1))
 return output
lotteries = {1: Lottery('Nebraska Pick Three',
 [{'picks': 3, 'max': 9, 'repeats': True}]),
 2: Lottery('Nebraska Pick Five',
 [{'picks': 5, 'max': 38}]),
 3: Lottery('Powerball',
 [{'picks': 5, 'max': 75}, {'max': 15}])}
def play():
 """Allow the user to choose a lottery and simulate a draw."""
 print("Select a lottery:")
 for key in sorted(lotteries):
 print("{0}: {1.name}".format(key, lotteries[key]))
 while True:
 try:
 choice = int(input("Make a choice: "))
 except ValueError:
 print("Please enter an integer.")
 else:
 if choice in lotteries:
 break
 print("Not a valid choice.")
 print(lotteries[choice].draw())
if __name__ == "__main__":
 play()

And its output:

>>> play()
Select a lottery:
1: Nebraska Pick Three
2: Nebraska Pick Five
3: Powerball
Make a choice: foo
Please enter an integer.
Make a choice: 42
Not a valid choice.
Make a choice: 1
[1, 7, 9]
>>> play()
Select a lottery:
1: Nebraska Pick Three
2: Nebraska Pick Five
3: Powerball
Make a choice: 2
[34, 33, 20, 7, 35]
>>> play()
Select a lottery:
1: Nebraska Pick Three
2: Nebraska Pick Five
3: Powerball
Make a choice: 3
[75, 4, 12, 31, 55, 2]

Here I have used a simple dictionary implementation of the rules, with sensible defaults to minimise the input needed.

• Frank Tudor: Very cool! Ok, I am seeing the dictionary is a very viable alternative here. I was get else if'd out of my mind :)
• Frank Tudor: @jonsharpe Question: In my code there is something I am doing but I don't know what it means. Simplified example random.sample(range(1,50+1),5) That +1 (what does that mean?)
• jonrsharpe: @FrankTudor why are you doing it if you don't know what it means?! +1 is necessary because range is "half-open", i.e. excludes the final argument (e.g. range(10) gives 0, 1, ..., 8, 9 but not 10).
• Frank Tudor: @jonsharpe Ahhhh! Now this will happens even if I am explicit and say 1-50 (because I want 1-50) Python will do 0-49 unless I clip on that +1, is this assumption correct?
• jonrsharpe: @FrankTudor not quite, it's only half-open - the lower bound is included, so range(1, 50) would give 1-49 inclusive. See e.g. here for why. Rather than writing stuff without knowing why, work through a tutorial to get a more structured introduction - docs.python.org/3/tutorial/controlflow.html#the-range-function.