我决定使用一个名为guizero
的模块创建一个折扣口袋妖怪战斗模拟器。它基本上是一个更容易学习的Tkinter包装器。最近,我开始学习OOP,并认为这个项目将是我的“新发现的技能”的良好实践。我的计算机组中有人看了我的代码,告诉我这很糟糕,所以我想我会得到别人的意见。
我只想指出,我受过训练,可以将我编写的所有代码都注释成初学者能够理解的标准。
这是代码(您必须下载guizero才能使其工作):
# Import guizero for the GUI and random
import guizero as gui
import random
# Create an empty list for the Animals
actives = []
# Create an immutable list (tuple) for default animals
DEFAULT_ATTRS = (('Cow', 100, 10, 15, 4),
('Chicken', 40, 50, 40, 5),
('Duck', 45, 35, 70, 2))
# Create a function that checks if all values in a list are equal
def all_equal(iterable):
for i in range(len(iterable)):
if iterable[i-1] != iterable[i]:
return False
return True
# Create an Animal class for the animals
class Animal:
# Create an immutable value so that another values can't change the string
# Make a string that can be formated and evaluated
POWERSUM = 'int((({1}+{2})*{3})/{0})'
def __init__(self,name=None,strength=None,speed=None,skill=None,age=None):
assign = all_equal([name,strength,speed,skill,age]) and name == None
self.assigned = not assign
if assign:
return None
self.optimum = 50
self.name = name.title()
attr = [strength,speed,skill]
while sum(attr) > random.randint(180,220):
# If the sum is greater than 220 (or less)
# Change the max and the min values
attr[attr.index(max(attr))] -= random.randint(1,9)
attr[attr.index(min(attr))] += random.randint(1,5)
self.strength, self.speed, self.skill = attr
self.fitness = 100
self.attr = attr[:]
self.active = True
# Create a list with the values [number of battles, battles lost]
self.battles = [0,0]
self.age = age
self.power = 0
def __repr__(self):
# Create the display string
# Name. Stats: Strength, Spped, Skill and Age
attr = self.attr[:]+[self.age]
return '{}. Statistics: {},{},{} and {}'.format(self.name,*attr)
def returnPower(self):
# Get the power. The optimum age is 50
# Effectively create a parabola effect on the age
if self.age > 50:
age = self.optimum / (101-self.age)
else:
age = self.optimum / self.age
self.power = eval(self.POWERSUM.format(age,*self.attr))
return self.power
# Add the three default values
for attr in DEFAULT_ATTRS:
actives.append(Animal(*attr))
class BattleWindow(gui.App):
# Create a class that creates a GUI,
# avoiding the need for global variables; they're all attributes
def __init__(self,*animals):
super().__init__(title='Animals Battles',layout='grid')
# Create the function so that if the window is closed,
# it automatically opens the menu window
self.on_close(self.cancel)
texts = [[],[]]
for i,person in enumerate(['Animal Selected','Opponent']):
texts[i].append(person)
for cate in ['Strength','Skill','Speed','Age','Fitness','Power']:
texts[i].append(cate)
buttons = ((self.power, 'Power' ,[0,0]),
(self.opponent,'Opponent' ,[1,0]),
(self.battle, 'Battle' ,[2,0]),
(self.firstaid,'First aid',[3,0]))
for func,text,grid_xy in buttons:
self.aidbtn = gui.PushButton(self,func,text=text,grid=grid_xy)
self.animals = list(animals)
# Create 2 'empty' animals that can't do anything
# just in case the user tries to do something
self.chosen = Animal()
self.opponent = Animal()
self.displays = [[],[]]
self.options = ['None']
for animal in animals:
self.options.append(animal.name)
# Create a Combo to choose the animal
self.combo = gui.Combo(self,self.options,command=self.disp,grid=[0,2])
for i,text in enumerate(texts):
for x,tx in enumerate(text):
pos = [[x],[x]]
if i%2 == 0:
pos[0].append(1)
pos[1].append(2)
else:
pos[0].append(3)
pos[1].append(4)
gui.Text(self,text=tx+': ',grid=pos[0],align='left')
if tx != 'Animal Selected':
self.displays[i].append(gui.Text(self,grid=pos[1]))
# Display the GUI so that everything shows up
self.display()
def battle(self):
fitness = 'fitness'
if not (hasattr(self.chosen,fitness) or hasattr(self.opponent,fitness)):
gui.warn('No opponent!','You need an opponent!')
return
# Decrease the fitnesses of the animals by 75% of the value
self.opponent.fitness *= 0.75
self.chosen.fitness *= 0.75
# Add 1 to the number of battles
self.chosen.battles[0] += 1
self.opponent.battles[0] += 1
# If power has not yet been calculated,
# return so that the battle never happens
if self.displays[0][-1].get() == 'N/A':
return
if self.opponent.power > self.chosen.power:
winner = self.opponent
self.chosen.fitness *= 0.75
self.chosen.battles[1] += 1
else:
winner = self.chosen
self.opponent.fitness *= 0.75
self.chosen.battles[1] += 1
gui.info('The winner is...','The Winner is ... {}'.format(winner.name))
# Set the fitness display to the fitness to 2d.p.
self.displays[0][-2].set(round(self.chosen.fitness,2))
self.displays[1][-2].set(round(self.opponent.fitness,2))
# Check if either fitness is less than 1 as
# 0 can never be reached
if self.opponent.fitness < 1 or self.chosen.fitness < 1:
if self.opponent.fitness < 1:
self.opponent.active = False
name = self.chosen.name
popname = self.opponent.name
x = 1
if self.chosen.fitness < 1:
self.chosen.active = False
name = 'None'
popname = self.chosen.name
x = 0
# Clear the displays if the fitnesses are less than 1
for disp in self.displays[x]:
disp.clear()
# Remove the name from the dropdown options
# then destroy the combo and create a new one
# The new combo is then set to either the current
# animal or None if the user animal faints
self.options.remove(popname)
self.combo.destroy()
self.combo = gui.Combo(self,self.options,grid=[0,2])
self.combo.set(name)
# Get rid of the Animal object from the animals so that
# the random opponent can't be one of the fainted ones
actives.pop([i.name for i in actives].index(popname))
def cancel(self):
# Go back to the menu system
self.destroy()
Menu()
def disp(self,_):
# If the combo is None, set the displays to N/A
if self.combo.get() == 'None':
for disp in self.displays[0]:
disp.set('N/A')
self.chosen = self.animals[self.options.index(self.combo.get())-1]
# Create a copy of the attr attribute of self.chosen.attr
# Next add the age and the fitness to the list
attrs = self.chosen.attr[:]
attrs.append(self.chosen.age)
attrs.append(self.chosen.fitness)
# Next change the displays to the
# appropriate values
for i in range(len(attrs)):
self.displays[0][i].set(attrs[i])
# Finally set the 'Power' display to N/A
self.displays[0][-1].set('N/A')
def firstaid(self):
# Create a function that allows self.chosen to get more fitness
if self.chosen.battles[0] == 0:
return
# Check if the battle win percentage is high enough to get first aid
if 100 * (self.chosen.battles[1] / self.chosen.battles[0]) > 60:
if self.chosen.fitness > 50:
amount = 100 - self.chosen.fitness
else:
amount = 50
self.chosen.fitness += amount
self.displays[0][-2].set(round(self.chosen.fitness,2))
# Make the button disabled so that it can't be pressed again
self.aidbtn.config(state=gui.DISABLED)
else:
gui.warn('Too many losses','You haven\'t won enough battles!')
def opponent(self):
# Randomly choose an enemy. While that enemy
# is the same as the 'chosen', choose again
value = random.choice(actives)
while value == self.chosen:
value = random.choice(actives)
self.opponent = value
# Create a copy of the opponent attrs
# Then add the age, fitness and name
attrs = self.opponent.attr[:]
attrs.append(self.opponent.age)
attrs.append(self.opponent.fitness)
attrs.insert(0,self.opponent.name)
# Add the displays for the opponent
for i in range(len(attrs)):
self.displays[1][i].set(attrs[i])
self.displays[1][-1].set('N/A')
def power(self):
# Set the text to the power. Doesn't need
# the value to be assigned; happens in the returnPower() function
if self.chosen.assigned:
self.displays[0][-1].set(self.chosen.returnPower())
if self.opponent.assigned:
self.displays[1][-1].set(self.opponent.returnPower())
# Create the default window that creates
# a menu system
class Menu(gui.App):
def __init__(self):
super().__init__(title='Menu System',layout='grid',height=300)
gui.Text(self,text='Please choose an option',grid=[0,0])
# Create a 2d tuple containing the infos
options = (('Add new animal',self.addNew,[1,0]),
('Battle!!!',self.battle,[2,0]),
('Delete animal',self.delete,[3,0]))
# Create a list containing the names of the
# animals for leter
self.names = [i.name for i in actives]
# Create the buttons for the options
for text,func,grid_xy in options:
gui.PushButton(self,func,text=text,grid=grid_xy)
# Display all the widgets from the GUI
self.display()
def clear(self):
# Clear the texts used
for text in self.text:
text.destroy()
# Clear the entries used
for ent in self.entries:
ent.destroy()
# Clear and delete the 2 buttons
self.btn.destroy()
self.cancel.destroy()
del self.btn,self.cancel
def addAnimal(self):
# Create a list of the gotten values
self.got = []
for i in range(len(self.entries)):
self.got.append(self.entries[i].get())
if self.got[0] == '':
gui.error('Name','Please provide a name')
return
# Add the animal to the actives values
actives.append(Animal(*self.got))
gui.info('Animal Added','{} added!'.format(self.got[0]))
# Clear the widgets
self.clear()
def addNew(self):
# Create a tuple containg the Text widget information
texts = (('strength',[1,2],[1,1]),
('speed', [2,2],[2,1]),
('skill', [3,2],[3,1]),
('age', [4,2],[4,1]))
entries = []
text = []
text.append(gui.Text(self,text='Enter animal name: ',grid=[0,1]))
entries.append(gui.TextBox(self,grid=[0,2],width=25))
# Create the Text widgets and the Slider widgets
for t,sc,tc in texts:
text.append(gui.Text(self,text='Enter animal '+t+': ',grid=tc))
entries.append(gui.Slider(self,start=1,end=100,grid=sc))
# Create copies of the entries and text lists
self.entries = entries[:]
self.text = text[:]
# Create the 2 buttons for submitting and cancelling
self.btn = gui.PushButton(self,self.addAnimal,text='Submit',grid=[6,1])
self.cancel = gui.PushButton(self,self.clear,text='Cancel',grid=[6,2])
def battle(self):
# Destroy menu window and open BattleWindow
self.destroy()
BattleWindow(*actives)
def deleteOne(self):
# If the combo for deletion does not equal None
# pop the name from actives and give a info window
if self.todelete.get() != 'None':
index = self.names.index(self.todelete.get())
delete = actives.pop(index).name
gui.info('Deleted','{} has been deleted!'.format(delete))
self.names.pop(index)
# Destroy the button and Combo
self.todelete.destroy()
self.bn.destroy()
def delete(self):
# Create a combo for the animal and a 'delete' button
self.todelete = gui.Combo(self,['None']+self.names,grid=[0,1])
self.bn = gui.PushButton(self,self.deleteOne,text='Delete',grid=[1,1])
# Initialize the menu window to start
Menu()
发布于 2017-03-21 11:02:15
# Import guizero for the GUI and random
import guizero as gui
import random
和你的许多评论一样,这正是你永远不应该写的评论。上面只写着密码是干什么的。我们可以看到代码的作用。在绝对必要的情况下,使用注释来解释为什么会这样做。你补充说
我受过训练,把我编码的每一件事都写成初学者能理解的标准
但是,我不认为在这方面注释任何东西都有帮助,与help
中将出现的具有合理变量名和良好的文档字符串的编写良好的代码相比。
此外,您还应该对导入进行分组,首先是标准库,每个风格指南。
import random
import guizero as gui
# Create an immutable list (tuple) for default animals
DEFAULT_ATTRS = (('Cow', 100, 10, 15, 4),
('Chicken', 40, 50, 40, 5),
('Duck', 45, 35, 70, 2))
一件小事,但元组不仅仅是一个不可变的列表;参见列表和元组有什么区别?
此外,我不会像这样对齐值;在每个逗号之后使用一个空格,否则如果添加另一个名称或值较长的项,则必须重新对齐所有内容。
# Create a function that checks if all values in a list are equal
def all_equal(iterable):
这个注释又是多余的,更糟糕的是,它与代码不一致。显然,您已经意识到此功能在非列表迭代器中运行良好,并重新命名了参数,但您还没有更新注释。此外,当您描述模块、类和函数时,您应该使用docstring,而不仅仅是注释:
def all_equal(iterable):
"""Whether all of the values in the iterable are equal."""
这使得它们对IDE、文档生成器等都很有用。
assign = all_equal([name,strength,speed,skill,age]) and name == None
self.assigned = not assign
if assign:
return None
这很糟糕,对不起,没有两种办法。如果这是为了验证输入,因此必须至少提供一个值,则应该如下所示:
if all(item is None for item in [name,strength,speed,skill,age]):
raise ValueError('at least one of the inputs must be provided')
您可以使用any
而不是all
来表示“必须提供所有输入”,但是在这种情况下,为什么要提供默认的参数值呢?注意None
通过标识(is
)而不是相等(==
)的比较;它是单例的。
一般来说,你的初始化似乎太长,太复杂了。具体来说,我想提出以下几点:
attr = [strength,speed,skill]
while sum(attr) > random.randint(180,220):
# If the sum is greater than 220 (or less)
# Change the max and the min values
attr[attr.index(max(attr))] -= random.randint(1,9)
attr[attr.index(min(attr))] += random.randint(1,5)
self.strength, self.speed, self.skill = attr
将是:
self.strength, self.speed, self.skill = self._adjust_attr_values(strength, speed, skill)
同样,这个方法将有一个docstring来解释为什么这是必要的。
我不会存储self.attr
;这会重复现有的信息,并且有更新其中一个而不是另一个的风险。如果真的需要它,它应该是一个经过计算的、理想的只读属性:
@property
def attr(self):
return [self.strength, self.speed, self.skill]
# Create a list with the values [number of battles, battles lost]
self.battles = [0,0]
这方面的一个问题是,列表实际上并没有保留它的内容。您会发现自己正在编写battles, lost = thing.battles
,即使您只需要其中之一,然后您就会将订单混在一起,您会发现有一个非常棘手的bug需要跟踪。为什么不具有两个属性呢?
self.battles_won = 0
self.battles_lost = 0
您可以为总数添加另一个属性。或者完全创建一个新的对象来保持与总数的输赢,但这可能是过头了。
通常,我还会首先分配所有参数,然后根据固定值进行所有初始化。这意味着读者可以尽早把参数的上下文从他们的脑子里拿出来。
def __repr__(self):
# Create the display string
# Name. Stats: Strength, Spped, Skill and Age
attr = self.attr[:]+[self.age]
return '{}. Statistics: {},{},{} and {}'.format(self.name,*attr)
根据数据模型,__repr__
应该:
...look类似于一个有效的Python表达式,该表达式可以用于重新创建具有相同值的对象(给定适当的环境)。如果这是不可能的,则应该返回表单
<...some useful description...>
的字符串。
您的方法两者都不做,因此应该称为__str__
。
def returnPower(self):
# Get the power. The optimum age is 50
# Effectively create a parabola effect on the age
if self.age > 50:
age = self.optimum / (101-self.age)
else:
age = self.optimum / self.age
self.power = eval(self.POWERSUM.format(age,*self.attr))
return self.power
基于上述所有情况:
@property
def power(self):
"""Calculate the power. The optimum age is 50."""
age = 50 / ((101 - self.age) if self.age > 50 else self.age)
return int(((self.strength + self.speed) * self.skill) / age)
这比您的公式要复杂得多;我不知道您试图用POWERSUM
减轻什么风险,如果所有的动物都有相同的optimum
,为什么要将它作为实例属性呢?
# Create an empty list for the Animals
actives = []
# Create an immutable list (tuple) for default animals
DEFAULT_ATTRS = (('Cow', 100, 10, 15, 4),
('Chicken', 40, 50, 40, 5),
('Duck', 45, 35, 70, 2))
...
# Add the three default values
for attr in DEFAULT_ATTRS:
actives.append(Animal(*attr))
回顾过去,这一切似乎有点奇怪。为什么不定义类然后做:
actives = [
Animal('Cow', 100, 10, 15, 4),
Animal('Chicken', 40, 50, 40, 5),
Animal('Duck', 45, 35, 70, 2),
]
发布于 2017-03-21 13:01:10
您的大多数评论并不是很好,但感谢您将它们包括在内,因为有些评论在更改代码时有所帮助。当您的代码很难理解或阅读时,注释是很好的,因此,如果您优化了一些内容,则需要包含注释。因此,在做了几次修改之后,我唯一要保留的评论就是returnPower
中的那个。
但是您的代码相当不错,在逐行级别上,您的代码是相当不错的。但是,您的代码作为一个整体结构很差。您希望尽可能多地将代码从GUI类加载到其他类。在这种情况下,Battle
类将是一个很好的补充。你需要一个函数来显示关于你的战斗的所有信息,这样你就可以在每次对战斗的改变之后调用一个函数。
self.chosen
设置为None
。random
是不好的,因为它不允许开发人员准确地保存动物状态。相反,在游戏的每一负荷下,动物可能会改变。如果您想实现这一点,那么在类之外执行它,比如创建一个新的动物。returnPower
成为一个属性,并删除power
,因为将函数重命名为power
允许您随意使用它。firstaid
时,只需要检查类是否处于正确的状态,然后调用min
,而不是很难读取if/else。change_opponent
应该只是改变对手,并确保新的对手是不一样的,目前选定的动物。_build_
为前缀。我会将它们分成以下几组:按钮、下拉列表、stat_labels、display和opponent_label。display_battle
函数,这将更新所有动物的统计数据,以及对手的名字。这些都没有改变,如果您正确地更新了所有内容,您就不必在其他任何地方实现这一点。battle
、firstaid
和opponent
都变得简单得多,您可以调用Battle函数,并显示任何输出。因此,我将使用以下类:
class Animal:
POWERSUM = 'int((({1}+{2})*{3})/{0})'
def __init__(self, name, strength, speed, skill, age):
self.name = name.title()
self.strength = strength
self.speed = speed
self.skill = skill
self.age = age
self.optimum = 50
self.fitness = 100
self.battles = [0, 0]
def __str__(self):
return ('{} ({}, {}, {}, {})'.format(self.name, self.strength,
self.speed, self.skill, self.age))
@property
def power(self):
# Get the power. The optimum age is 50
# Effectively create a parabola effect on the age
if self.age > 50:
age = self.optimum / (101-self.age)
else:
age = self.optimum / self.age
return eval(self.POWERSUM
.format(age, self.strength, self.speed, self.skill))
def fight(self, other):
for animal in (self, other):
animal.fitness *= 0.75
animal.battles[0] += 1
winner, looser = (
(self, other)
if self.power > other.power else
(other, self)
)
winner.battles[1] += 1
looser.fitness *= 0.75
for animal in (self, other):
if animal.fitness < 1:
animal.active = False
return winner, looser
class Battle:
def __init__(self, animals):
self.animals = list(animals)
self.chosen = None
self.opponent = None
def battle(self):
if self.chosen == self.opponent:
return False, "Animals can't fight themselves"
if self.chosen is None or self.opponent is None:
return False, "One or more animals have not been selected"
ret = self.chosen.fight(self.opponent)
if self.chosen.fitness < 1:
self.animals.remove(self.chosen)
self.chosen = None
if self.opponent.fitness < 1:
self.animals.remove(self.opponent)
self.change_opponent()
return True, ret
def firstaid(self):
if self.chosen is None:
return False, 'Animal is None'
if self.chosen.battles[0] == 0:
return False, 'No battles'
if 100 * (self.chosen.battles[1] / self.chosen.battles[0]) <= 60:
return False, "You haven't won enough battles!"
self.chosen.fitness = min(self.chosen.fitness + 50, 100)
return True, None
def change_opponent(self):
choices = [a for a in self.animals if a != self.chosen]
if choices:
value = random.choice(choices)
else:
value = None
self.opponent = value
class BattleWindow(gui.App):
def __init__(self, *animals):
super().__init__(title='Animals Battles', layout='grid')
self._battle = Battle(animals)
self.on_close(self._cancel)
self._buttons = self._build_buttons()
self._dropdown = self._build_dropdown()
self._labels = self._build_stat_labels()
self._display = self._build_display()
self._opponent = self._build_opponent_label()
# Default enemy, and update view
self.opponent()
self.display()
def _cancel(self):
# Go back to the menu system
self.destroy()
Menu()
def _build_buttons(self):
buttons = (
(self.battle, 'Battle', [0, 0]),
(self.firstaid, 'First aid', [1, 0]),
# (self.opponent, 'Opponent', [2, 0])
)
return [
gui.PushButton(self, func, text=text, grid=grid_xy)
for func, text, grid_xy in buttons
]
def _build_dropdown(self):
options = ['None'] + [animal.name for animal in self._battle.animals]
return gui.Combo(self, options, command=self._dropdown_change,
grid=[0, 2])
def _build_stat_labels(self):
cols = ['Strength', 'Skill', 'Speed', 'Age', 'Fitness', 'Power']
labels = []
for i, person in enumerate(['Animal Selected', 'Opponent']):
i = i*2 + 1
labels.append([
gui.Text(self, text=txt+': ', grid=[j, i], align='left')
for j, txt in enumerate([person] + cols)
])
return labels
def _build_display(self):
display = []
for i in range(2):
i = i*2 + 2
display.append([
gui.Text(self, grid=[j, i])
for j in range(1, 7)
])
return display
def _build_opponent_label(self):
return gui.Text(self, grid=[0, 4])
def _dropdown_change(self, _):
chosen = self._dropdown.get()
if chosen == 'None':
chosen = None
else:
possible = (a for a in self._battle.animals if a.name == chosen)
chosen = next(possible, None)
self._battle.chosen = chosen
self.display_battle()
def display_battle(self):
battle = self._battle
self._opponent.set('None'
if battle.opponent is None else
battle.opponent.name)
animals = [battle.chosen, battle.opponent]
for disp, animal in zip(self._display, animals):
if animal is None:
for d in disp:
d.set('N/A')
else:
disp[0].set(animal.strength)
disp[1].set(animal.skill)
disp[2].set(animal.speed)
disp[3].set(animal.age)
disp[4].set(round(animal.fitness, 2))
disp[5].set(animal.power)
def battle(self):
status, ret = self._battle.battle()
if not status:
gui.warn('Battle', ret)
else:
gui.info('Battle', 'The Winner is {}'.format(ret[0].name))
self.display_battle()
def firstaid(self):
status, ret = self._battle.firstaid()
if not status:
gui.warn('Firstaid', ret)
self.display_battle()
def opponent(self):
self._battle.change_opponent()
self.display_battle()
https://codereview.stackexchange.com/questions/158371
复制相似问题