HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Spin-the-bottle-like game, follow-up

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thebottlespinlikegamefollow

Problem

I've posted this some time ago. Here is the new version that I made based on all the comments that were made. Another look would be much appreciated, especially on:

-
Getting rid of the global statements

-
I read about the use of the return statement which is bad practice in some situations (see code). Is that correct?

-
The if elif else way of coding the menu (at the bottom) which was proposed to be put in a list and then calling the list items. I did not manage to get the first option of actually playing the game to work.

```
import random
import time
from itertools import cycle

class Die:
def __init__(self, name):
self.name = name
self.options = []
self.pick = 0

def add(self):
print('Enter {0} and press enter. Enter "q" when done.\n'.format(self.name))

while True:
additem = input('Add: ')
if additem.lower() == 'q':
break
elif additem == '':
continue
else:
self.options.append(additem.lower().replace(' ', '_'))

def roll(self):
self.pick = random.choice(self.options)

def addactions():
print('Add actions to the dice. Enter "q" when done')
while True:
nextitem = input("Add: ")
if nextitem == '':
continue
elif nextitem.lower() == 'q':
break
else:
actionDie.options.append(nextitem.lower().replace(' ', '_'))

def addbp():
print('Add body parts to the dice. Enter "q" when done')
while True:
nextitem = input("Add: ")
if nextitem == '':
continue
elif nextitem.lower() == 'q':
break
else:
bpDie.options.append(nextitem.lower().replace(' ', '_'))

def load():
# Load each one in its correct list and players in each variable
# What is already in the lists will still be there
while True:
loadfile = input('Enter file name to LOAD (includi

Solution

Quick answer but the part in the with open(loadfile, 'rt') as file: block
definitly looks wrong to me. Here are a few suggestions.

You shouldn't use the same variable for different purposes :

newitem = ''
newitem = newitem.join(line)
some_function(newitem)


What is newitem ? Is it a separator or a concatenation of values ? You can simply write this :

some_function(''.join(line))


Also, the way line is sometimes a string sometimes a list can lead to confusion.

If my understanding is correct, you have a list of looking like this : "some_kind_of_key: some kind of values". At the moment, the issue is that you are looking for the key on the whole line so that if you were to be fed a line like "bodypart: action:", this would be handled as if it was the file "action: bodypart:" which is very confusing. What you need to do is to perform some processing of the first item of the list :

Your code becomes :

for str_line in file:
                list_line = str_line.split()
                if list_line:  # nothing to do for empty lines (?)
                    key = list_line[0]
                    values = ''.join(list_line[1:])
                    if key == 'action:':
                        actionDie.options.append(values)
                    elif key == 'bodypart:':
                        bpDie.options.append(values)
                    elif key == 'player1:':
                        global player1
                        player1 = values
                    elif key == 'player2:':
                        global player2
                        player2 = values
                    else:
                        print("ERROR: item", key, "not loaded")


There are many other things to change but I'll leave this to others.

Code Snippets

newitem = ''
newitem = newitem.join(line)
some_function(newitem)
some_function(''.join(line))
for str_line in file:
                list_line = str_line.split()
                if list_line:  # nothing to do for empty lines (?)
                    key = list_line[0]
                    values = ''.join(list_line[1:])
                    if key == 'action:':
                        actionDie.options.append(values)
                    elif key == 'bodypart:':
                        bpDie.options.append(values)
                    elif key == 'player1:':
                        global player1
                        player1 = values
                    elif key == 'player2:':
                        global player2
                        player2 = values
                    else:
                        print("ERROR: item", key, "not loaded")

Context

StackExchange Code Review Q#52228, answer score: 4

Revisions (0)

No revisions yet.