Gigantic class to model a baseball game

I'm trying to think of ways to separate things out. I'm open to ideas, or if you see anything blatantly wrong, I'd like to know that too.

Generally, I'm happy with this, but the sheer size of the class seems like a code smell.

Original Source

```
'''The Game class.'''
import game_exceptions
from bases import Bases
from count import Count
from inning import Inning
from team import Team

# pylint: disable=R0902
# pylint complains about the number of instance attributes.
class Game(object):
'''Maintain the state of the game.

Includes methods to carry out various game
related events.

'''
def __init__(self, home, away, league="league.db", innings_per_game=9):
'''Set up a new game.

home and away are the team_id pointing to a team in the league.

The league variable is used to play games in different leagues.

innings_per_game is the number of innings in a standard
game, it can be changed if you want to represent a
little league game, or some type of exhibition game.

'''
self.count = Count()
self.home = Team(league=league, team_id=home)
self.away = Team(league=league, team_id=away)
self.inning = Inning(innings_per_game)
self.bases = Bases()
self.winner = None
self.game_over = False
self.bat_team = self.away
self.pitch_team = self.home
self.pitch_team_stats = self.pitch_team.stats.pitching
self.current_pitcher = self.pitch_team.pitcher().stats.pitching
self.bat_team_stats = self.bat_team.stats.batting
self.current_batter = self.bat_team.current_batter().stats.batting

def __str__(self):
'''Output the important game information as a string.'''
if self.game_over:
return self.__str_finished_game()
else:
return self.__str_game_in_progress()

def __str_finished_game(self):
'''Output a finished game as a string.'''
output =

Don't pass in the name of your database file. You should pass in a database connection object. As it stands you reopen the database in many different places across your code. You should really only open the database once and pass the connection around. If you do this:

• Your code will be simplified

• Your code will be more efficient

• You'll be able to run your unittests on the :memory: database which will make them faster.

Extract a GameStats() object. All those __stats_* methods should be methods on that object. The fact that you have a bunch of methods with those prefixes is a big hint. A sizable chunk of the Game class seems to be concerned with those stats, and so its a good candidate for extraction.

Don't use __str__ for output. It's supposed to be a string representation of your object, not your user output. I'd actually pull all user output out of this (and your other classes). Its better if you have some objects purely dedicated to the logic of he game. Have completely separately classes to do the user output.

Much of logic in Game should really be on Inning or Count. For example, the strike method.

def strike(self):
    '''Pitcher throws a strike.

    If there are 2 strikes, call the __strike_out method.
    Otherwise, just increment the strike count.

    '''
    if self.count.strikes == 2:
        self.__strike_out()
    else:
        self.count.strike()

Much of the logic here is concerned with the number of strikes which is Count's domain. This function is thus calling out to be moved to Count. But you can't because you need to call __strike_out. You've got the relationship between Count and Game backwards. Count should have a reference to and call Game when there is a strike out.

StackExchange Code Review Q#20358, answer score: 4