this is a good start, there are a couple of bugs and a couple of places where we can make the code more concise. In order to make the code simpler (and thus less likely to have bugs) we will look at snippets of code near each other and check how similar they are. Anywhere we find blindingly obvious repeats, we will try and make a function and make it do the work.
I won't make too many comments on style or formatting, if you want to find out more yourself a good place to start is pep8 or run your code through a linter and start googling the lint warnings. Some of the stuff to look at would be unnecessary brackets and comments that don't add much value. I'll do my best to make any code I produce here follow best practices.
All the code at the start deals with asking the user for input and validating the input.
if choice == ('R'):
one = random.randint(1,69)
two = random.randint(1,69)
three = random.randint(1,69)
four = random.randint(1,69)
five = random.randint(1,69)
pball = random.randint(1,26)
There is nothing special about one, two, three, four, or five. pball might be dealt with differently later, but it isn't all that special either. So since we never need to distinguish them, lets put them into a list.
if choice == "R":
balls = [None, None, None, None, None, None]
balls[0] = random.randint(1,69)
balls[1] = random.randint(1,69)
balls[2] = random.randint(1,69)
balls[3] = random.randint(1,69)
balls[4] = random.randint(1,69)
balls[5] = random.randint(1,26) # The powerball is in the last slot.
This seems fairly repetitious, we can instead use list syntax.
if choice == 'R':
# The powerball is in the last slot.
balls = [random.randint(1,69), random.randint(1,69), random.randint(1,69), random.randint(1,69), random.randint(1,69), random.randint(1,26)]
This is still really repetitive (and actually a bit too long too), we can use a list comprehension to save some typing.
if choice == "R":
balls = [random.randint(1,69) for _ in range(5)]
powerball = random.randint(1, 26)
elif choice == ('P'):
one = int(raw_input("Please pick your 1st number, between 1 and 69."))
two = int(raw_input("Please pick your 2nd number, between 1 and 69."))
three = int(raw_input("Please pick your 3rd number, between 1 and 69."))
four = int(raw_input("Please pick your 4th number, between 1 and 69."))
five = int(raw_input("Please pick your 5th number, between 1 and 69."))
pball = int(raw_input("Please pick your Powerball number, between 1 and 26."))
We have the same opportunity here to reduce the amount of code. This is a little more finicky because of the cardinal counting (1st, 2nd, etc) but I think it would be worth doing.
elif choice == 'P':
message = "Please pick your {} number, between 1 and 69 (inclusive)."
balls = []
for s in ("1st", "2nd", "3rd", "4th", "5th"):
ball = int(raw_input(message.format(s)))
balls.append(ball)
powerball = int(raw_input("Please pick your powerball number, between 1 and 26 (inclusive)."))
The second last line is a little too long. Another issue we haven't fixed is handling users who don't enter valid numbers. See if you can come up with a way to fix these problems. For the latter one try-except is the way to go.
if ((one or two or three or four or five) > 69) or pball > 26 or ((one or two or three or four or five or pball) < 1): #In case the entered numbers are too high or too low.
print "Please choose a number between 1 and 69."
This doesn't actually validate numbers correctly. Let's break it down the way python would read it.
(one or two or three or four or five)
This will check each number is "truthy". For ints, this is checking if the number is nonzero. So if any of the numbers are not zero (one or two or three or four or five) becomes True.
((one or two or three or four or five) > 69) # This becomes
(True > 69) # True is NOT greater than 69
False
So we need to fix this. Latter in the statement
... or ((one or two or three or four or five or pball) < 1)
(True < 1) # Assuming at least one of the numbers is nonzero.
False # Maybe surprisingly, True == 1.
So if we look at the overall if statement
if ((one or two or three or four or five) > 69) or pball > 26 or ((one or two or three or four or five or pball) < 1):
if False or pball > 26 or False:
if pball > 26: # False or X is the same as X.
So we need to check every number individually. We can write this very cleanly using the built in function 'any'
if any(ball > 69 for ball in balls) or powerball > 26 or any(ball < 1 for ball in balls):
This highlights two things, we never check if the powerball is less than 1, and we keep using these "magic" numbers. We should add that check, and make some variables for these numbers.
# Somewhere near the top of the code
MIN_BALL_VALUE, MAX_BALL_VALUE = 1, 69
MIN_POWERBALL_VALUE, MAX_POWERBALL_VALUE = 1, 26
I'll change any magic numbers to use these variable from now on.
So the validation check would look something like this:
if any(ball < MIN_BALL_VALUE or ball > MAX_BALL_VALUE for ball in balls) or\
powerball < MIN_POWERBALL_VALUE or powerball > MAX_POWERBALL_VALUE:
elif choice != ('R' or 'P'): #In case invalid input is given.
One small change, this can just be an else statement. It is easy to get (boolean) logic wrong so if you ever get the chance to write something as simple as "else", take it.
def powerball(human,real,jackpot,multiplier):
What do these arguments do? Spend 3 minutes writing a (short) docstring and other people who read your code will thank you. The supplied ones are not the best, hopefully it shows you some things you can add to the
def powerball(human,real,jackpot,multiplier):
"""Check if the balls passed won the powerball. Return a message with the result.
human: List[Int], the user's choice of balls. Assumes the last ball is the powerball.
real: List[Int], the winning balls.
jackpot: Int, the best prize possible.
multiplier: Int, if a prize is won, multiplies it.
Examples:
powerball([1, 2, 3, 4, 5, 6], [1, 2, 3, 4, 5, 6], 10, 2) ->'Won the jackpot of 10 * 2 == 20!'
powerball([1, 2, 3, 4, 5, 6], [11, 12, 13, 14, 15, 16], 10, 2) -> 'You get nothing!'"""
...
x = raw_input("Do you want to buy the multiplier for $1? ('Y' or 'N')")
if x == 'Y':
pass
elif x == 'N':
multiplier = 1
This is asking the user for input, and changing the multiplier based on it. It is rather unexpected to have this inside this function and away from the other user input. I would suggest moving it up to the other code that deals with user input.
p = 0 #Prize
i = 0 #Index
m = 0 #Matches
Rather than having a single word comment with what the variable is, just use the word. They are great names, and it is free to use more characters in variable names.
prize = 0
index = 0
matches = 0
if human[5] == real[5]: #If you only get the powerball right.
p = 4
for i in range(0,5): #If you only get normal numbers, no powerball.
...
for i in range(0,5): #If you get normal numbers along with the powerball.
...
The code here looks a little strange. I would avoid changing the iterating variable (i) inside a loop. We also have some repeated code to reduce. I won't delve into this code (apologies!) as I don't think I'd correctly root out all the bugs.Instead I will offer an alternative implementation, which should hopefully highlight some test cases to watch out for.
We want to count the number of unique matches. There are some annoying cases like human = [1, 1, 1, 1, 1, 1], real = [1, 2, 3, 4, 5, 1] which shouldn't be allowed. So we are going to be defensive and check everything. To make life easier, take the powerball out of both sets of balls.
balls, powerball = human[:-1], human[-1]
# In python3 you could write *balls, powerball = human
if len(set(balls)) != len(balls):
raise ValueError("The balls have duplicates!")
winning_balls, winning_powerball = real[:-1], real[-1]
if len(set(winning_balls)) != len(winning_balls):
raise ValueError("The winning balls have duplicates!")
matches = sum(ball in winning_balls for ball in balls)
matched_powerball = powerball == winning_powerball
We can count the matches by a nice sum. We get 1 if a ball is in the set of winning balls and 0 otherwise. So summing them gives the total number of matches.
prizes = {
# (matches, matched_powerball): prize
(3, False): 7,
(4, False): 100,
(5, False): 1000000,
(1, True): 4,
(2, True): 7,
(3, True): 100,
(4, True): 50000,
(5, True): jackpot}
if (matches, matched_powerball) in prizes:
prize = prizes[(matches, matched_powerball)]
elif matched_power_ball:
prize = 4
p *= multiplier
if p:
return "Congrats, ..."
return "Sorry, ..."
A couple of final points, the prizes dictionary is mostly static and could be moved outside of the function. That's a good candidate for either a global variable or some other method of configuration management. Notice how the final lines no longer have an if-elif statement, just if. This is the same reasoning as above, if you can rewrite your code with less elifs, it means there are less chances to introduce bugs.
