trjhtr

I have made this program which simulates a slot machine.

import random, time, os

def input_verify(message):
 while True:
 user_input = input(message).lower()
 if user_input in ("yes", "no"):
 return user_input
 print("Invalid Input")

def win(number_of_matches, fruit):
 global balance
 print("Well done! You got", number_of_matches, "of the same fruit!")
 time.sleep(0.5)
 reward = fruits[fruit][1] * number_of_matches
 print("You won £!".format(reward))
 balance += reward

fruits = "apple": [10000, 1], "banana": [7000, 1.5], "lemon": [6000, 1.75], "orange": [5000, 2], "kiwi": [4000, 4],
 "peach": [3000, 6], "avocado": [2000, 10], "grapes": [1000, 20], "mango": [500, 100], "melon": [1, 10000]
weighted_fruits = [fruit for fruit in fruits for i in range(fruits[fruit][0])]

print("Welcome to my slot machine! Each turn costs £1.")
balance = 10
if os.path.exists("One Armed Bandit.txt"):
 with open("One Armed Bandit.txt", "r") as f:
 contents = f.readlines()
 continue_saved_game = input_verify("Do you want to continue your saved game?")
 if continue_saved_game == "yes":
 balance = float(contents[1].strip())

time.sleep(0.5)
print("nYour current balance is £.n".format(balance))
time.sleep(1)
while True:
 balance -= 1
 reward = 0
 a, b, c = [random.choice(weighted_fruits) for i in range(3)]
 print("Spinning...")
 for fruit in (a, b, c):
 time.sleep(0.5)
 print(fruit)
 time.sleep(0.5)
 if a == b == c:
 win(3, a)
 elif a == b or a == c or b == c:
 if b == c:
 win(2, c)
 else:
 win(2, a)
 else:
 print("Unlucky. None of your fruits matched each other.")
 time.sleep(0.5)
 print("nYour current balance is £.n".format(balance))
 time.sleep(0.5)
 replay = input_verify("Do you want to play again?")
 if replay == "no":
 save = input_verify("Do you want to save your game?")
 if save == "yes":
 name = input("What is you name?")
 with open("One Armed Bandit.txt", "w") as f:
 f.write("'s Balance:n".format(name, balance))
 break

I don't like how there are so many time.sleep() calls scattered throughout my code. Is there a way to achieve the same thing but in a cleaner way. Moreover, could you suggest some ways to make it more efficient?

Thanks for sharing your code,

You should write code to adhere to the PEP8 standard, for the most part you've done that correctly, you have snake_case variable and function names, but you've put all your imports on one line.

import random, time, os

should become

import random
import time
import os

Give one line per import.

Your code lacks a main guard. If I was to import any functions or classes from your current code, your slot machine game would start! Generally you just want to wrap your main code in a conditional check that looks like

if __name__ == "__main__":
 main() 

where main is a function that starts the execution of your program.

A pattern you use in your code is relying on a string value ("yes" or "no") to determine which function executes. This is standard enough and nothing really wrong with it, but I would recommend instead of checking for this string literals, to use boolean values instead which are taken from shorter helper functions. A quick example.

You offer the ability to save and load game state, this is a good thing. However, you save the state in a Human readable fashion - i.e. "Bob's Balance: 24". Unless you're relying on this format somewhere else or even just displaying it, you could just save a single number in a file. No need to record a name.

The main code does a lot, you could break this up into helper functions. Some possible function names that jump out are.

load_game, save_game, wants_to_save, wants_to_replay

A pattern you use in your code is relying on a string value ("yes" or "no") to determine which function executes. This is standard enough and nothing really wrong with it, but I would recommend instead of checking for this string literals, to use boolean values instead which are taken from shorter helper functions. A quick example.

Your current code for prompting for saving the game

 replay = input_verify("Do you want to play again?")
 if replay == "no":
 save = input_verify("Do you want to save your game?")
 if save == "yes":
 name = input("What is you name?")
 with open("One Armed Bandit.txt", "w") as f:
 f.write("'s Balance:n".format(name, balance))
 break

I would create some helper functions that look like

def wants_to_replay():
 return input_verify("Do you want to play again?") == "yes"

def wants_to_save():
 return input_verify("Do you want to save your game?") == "yes"

def save():
 name = input("What is you name?")
 with open("One Armed Bandit.txt", "w") as f:
 f.write("'s Balance:n".format(name, balance))

And then the code becomes

 if not wants_to_replay():
 if wants_to_save():
 name = input("What is you name?")
 with open("One Armed Bandit.txt", "w") as f:
 f.write("'s Balance:n".format(name, balance))
 break

The presence of a global variable is a bit suspect. This usually indicates that maybe you should be using a class instead. Perhaps a SlotMachine object.

It might be nice to be able to instantiate a slot machine object and call a start method on it.

slot_machine = SlotMachine(saved_balance)
slot_machine.start()

this line,

a, b, c = [random.choice(weighted_fruits) for i in range(3)]

when there is an unused variable, you should indicate as such by naming it _
so this line becomes

a, b, c = [random.choice(weighted_fruits) for _ in range(3)]

But then a, b and c are not great variable names. They do have a small scope and are used immediately after, but there's no reason to not give them a better name.

Why not just call them all_fruit then in the next loop you can just use

for fruit in all_fruit:
 ...

In the next section you compare them to each other, you could simply then unpack these into variables like

fruit_a, fruit_b, fruit_c = all_fruit

I think single letter variable names (excluding _ and loop counters) should be avoided at all costs.

You currently have all the fruits values hard coded in, it might be worth while to extract that data into a file to make it easier to add/remove new ones. That's probably not a huge deal though and may have no real benefit.

I'm not sure if there's any cleaner way to achieve the sleeps without scattering the calls throughout the code. You could maybe use named constants for the duration, but there may not be much value in that either.

You have no docstrings in your code. The print statements act as a form of documentation however, and your variable names are (mostly) very good. But there may be some room for some useful comments.

Hopefully this review was useful for you!

@chatton already said that your fruits' characteristics should be stored in a separate file. I propose to read them in a list of namedtuples. For exmaple, your Fruit class definition will look like this:

from collections import namedtuple

Fruit = namedtuple('Fruit', ['name', 'weight', 'reward'])

And then after reading the fruits, you will have a list of Fruit instances like this:

fruits = [Fruit(name='apple', weight=10000, reward=1),
 Fruit(name='banana', weight=7000, reward=1.5),
 Fruit(name='lemon', weight=6000, reward=1.75),
 ...
 Fruit(name='melon', weight=1, reward=10000)]

weighted_fruits = [fruit for fruit in fruits for i in range(fruits[fruit][0])]
...
a, b, c = [random.choice(weighted_fruits) for i in range(3)]

You don't need this huge weighted_fruits list of thousands elements to get random items with weights. Instead you can use random.choices:

weights = [fruit.weight
 for fruit in fruits]
while True:
 selected_fruits = random.choices(fruits,
 k=3,
 weights=weights)
 ...

Use Counter to get fruits that were selected 2 or 3 times:

most_common_fruit, most_common_fruit_count = (Counter(selected_fruits)
 .most_common(n=1)[0])
if most_common_fruit_count == 1:
 print("Unlucky. None of your fruits matched each other.")
else:
 ...

Instead of a weirdly named function win you should have something like this:

def get_new_balance(original_balance: float,
 *,
 matches: int,
 reward: float) -> float:
 return original_balance + reward * matches

No globals (really bad practice), no printing (it should be outside) and supplying amount of reward directly instead of a fruit itself. I added type hints as well. It's a good practice to include them. You help yourself and other people by being explicit about what type of data should be supplied to and returned from a function. And asterisk means that all arguments that follow after are keyword-only now:

if most_common_fruit_count == 1:
 print("Unlucky. None of your fruits matched each other.")
else:
 balance = get_new_balance(balance,
 matches=most_common_fruit_count,
 reward=most_common_fruit.reward)

Some other advises:

1. Don't read a savefile before asking if user actually wants to read it.


2. 
   float(contents[1].strip()) - there is no need to strip here.


3. 
   reward = 0 is never used.


4. Avoid hardcoding things like 'One Armed Bandit.txt' or magic numbers like balance = 10. You could put them to a main function's signature as default parameters.


5. You could use f-strings if you are with Python 3.6: print(f"You won £reward!").


6. Try to follow PEP 8 style guide. @chatton already said about imports on separate lines. Also, don't exceed 79 characters on one line and separate functions by 2 blank lines.


7. 
   open("One Armed Bandit.txt", "r") - you can omit "r" as it is default parameter.