patternpythonMinor
Baseball game state class in python looking for feedback
Viewed 0 times
baseballlookinggamepythonstateforfeedbackclass
Problem
So far, this is just a class that tracks the state of a baseball game. Unfortunately, I can't tell you for sure what the next step is. It might be a an interactive playable baseball game, or a game simulator, or some combination.
I am hoping this class is sufficient to do either of those things. The test class wouldn't fit here. The code is hosted on bitbucket here as well. I'm looking for feedback from the pythonistas.
```
'''The Game class.'''
class Game(object):
'''Maintain the state of the game.
Includes methods to carry out various game
related events.
'''
# pylint: disable=R0902
# Ignore error about too many instance attributes.
def __init__(self):
'''Set up a new game.'''
self.count = {"balls": 0, "strikes": 0}
self.score = {"home": 0, "away": 0}
self.home_batter = 1
self.away_batter = 1
self.inning = {"val": 1, "half": "top"}
self.bases = {"first": 0, "second": 0, "third": 0}
self.winner = None
self.game_over = False
self.outs = 0
self.__new_inning()
def strike(self):
'''Pitcher throws a strike.'''
if self.count["strikes"] == 2:
self.__strike_out()
else:
self.count["strikes"] += 1
def ball(self):
'''Pitcher throws a ball.'''
if self.count["balls"] == 3:
self.__walk()
else:
self.count["balls"] += 1
def foul_ball(self):
'''Batter hits a foul.'''
if self.count["strikes"] != 2:
self.count["strikes"] += 1
def hit_by_pitch(self):
"Batter is hit by pitch."
self.__walk()
def steal(self, base):
'''Runner steals a base.
base - the base that is stolen (second, third, home)
If the base to be stolen is not empty, or the base
they are stealing from is not occupied, an error is
assumed to have occured, and nothing will happen.
'''
if base == "second":
if self.bases["first"] != 0 and self.bases["second"] == 0:
self.bases["second"] = 1
self.bases["first"] = 0
elif base
I am hoping this class is sufficient to do either of those things. The test class wouldn't fit here. The code is hosted on bitbucket here as well. I'm looking for feedback from the pythonistas.
```
'''The Game class.'''
class Game(object):
'''Maintain the state of the game.
Includes methods to carry out various game
related events.
'''
# pylint: disable=R0902
# Ignore error about too many instance attributes.
def __init__(self):
'''Set up a new game.'''
self.count = {"balls": 0, "strikes": 0}
self.score = {"home": 0, "away": 0}
self.home_batter = 1
self.away_batter = 1
self.inning = {"val": 1, "half": "top"}
self.bases = {"first": 0, "second": 0, "third": 0}
self.winner = None
self.game_over = False
self.outs = 0
self.__new_inning()
def strike(self):
'''Pitcher throws a strike.'''
if self.count["strikes"] == 2:
self.__strike_out()
else:
self.count["strikes"] += 1
def ball(self):
'''Pitcher throws a ball.'''
if self.count["balls"] == 3:
self.__walk()
else:
self.count["balls"] += 1
def foul_ball(self):
'''Batter hits a foul.'''
if self.count["strikes"] != 2:
self.count["strikes"] += 1
def hit_by_pitch(self):
"Batter is hit by pitch."
self.__walk()
def steal(self, base):
'''Runner steals a base.
base - the base that is stolen (second, third, home)
If the base to be stolen is not empty, or the base
they are stealing from is not occupied, an error is
assumed to have occured, and nothing will happen.
'''
if base == "second":
if self.bases["first"] != 0 and self.bases["second"] == 0:
self.bases["second"] = 1
self.bases["first"] = 0
elif base
Solution
# pylint: disable=R0902
# Ignore error about too many instance attributes.PyLint is trying to tell you something. Your class is to complex, and should be split into several smaller classes
def __init__(self):
'''Set up a new game.'''
self.count = {"balls": 0, "strikes": 0}Don't use dictionaries like this. You are using this dictionary like its an object with two attributes, "balls" and "strikes". What you should do is have a separate class called
AtBat or something which holds that data.self.score = {"home": 0, "away": 0}
self.home_batter = 1
self.away_batter = 1You've got two different pieces relating to the team. Instead, you should have a
Team object with score and batter attributes. self.inning = {"val": 1, "half": "top"}I'd identify an inning with a tuple
(1, True) for the top of the first, and (9, False) for the bottom of the ninth. Its awkward to check for strings.self.bases = {"first": 0, "second": 0, "third": 0}This should be a list
self.winner = None
self.game_over = False
self.outs = 0This is inning specific data, and suggests that you should have an inning class to hold it.
self.__new_inning()
def __put_out(self):
'''Runner is put out.'''
if self.outs == 2:
self.__side_retired()
else:
self.outs += 1
def __strike_out(self):
'''Batter strikes out.'''
self.__next_batter()
if self.outs == 2:
self.__side_retired()
else:
self.outs += 1The second function repeats the first, the second function should call the first one.
def __single(self):
'''Batter hits a single.'''
if self.bases["third"] != 0:
self.__score_run()
self.bases["third"] = 0
if self.bases["second"] != 0:
self.bases["second"] = 0
self.bases["third"] = 1
if self.bases["first"] != 0:
self.bases["first"] = 0
self.bases["second"] = 1
self.bases["first"] = 1
def __double(self):
'''Batter hits a double.'''
if self.bases["third"] != 0:
self.__score_run()
self.bases["third"] = 0
if self.bases["second"] != 0:
self.__score_run()
self.bases["second"] = 0
if self.bases["first"] != 0:
self.bases["first"] = 0
self.bases["third"] = 1
self.bases["second"] = 1These should be a single function which takes a numeric parameter 1 for a single, 2 for a double. You should be able to write a much simpler function using that.
Your function operate at differently levels of abstraction. You've got functions for the simplest parts of baseball like a
strike and then you've got functions for more complex things like triple_play. Your code should really have lower level functions for the non-batting portion as well. It should have functions like batter_tagged and ball_at_base. If you really need your higher level functions, they should probably be in a separate class and use the lower level functions to accomplish their goals.
Overall, your code is way more complicated than it needs to be. Hopefully, I've given you some ideas on how to improve it.
I'd suggest your class should operate something like this:
at_bat = game.next_at_bat()
at_bat.pitch(STRIKE)
at_bat.runner_moves(1,2) # steals base
at_bat.pitch(BALL)
at_bat.pitch(HIT)
at_bat.all_runners_forward()
at_bat.runner_tagged(2)
at_bat.runner_moves(1,2)
at_bat.ball_at_base(0)Code Snippets
# pylint: disable=R0902
# Ignore error about too many instance attributes.def __init__(self):
'''Set up a new game.'''
self.count = {"balls": 0, "strikes": 0}self.score = {"home": 0, "away": 0}
self.home_batter = 1
self.away_batter = 1self.inning = {"val": 1, "half": "top"}self.bases = {"first": 0, "second": 0, "third": 0}Context
StackExchange Code Review Q#11419, answer score: 2
Revisions (0)
No revisions yet.