Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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 == "third":
        if self.bases["second"] != 0 and self.bases["third"] == 0:
            self.bases["third"] = 1
            self.bases["second"] = 0
    else:
        if self.bases["third"] != 0:
            self.bases["third"] = 0
            self.__score_run()

def caught_stealing(self, base):
    '''Runner caught stealing a base.

    base - the base that is attempted to be 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"] = 0
            self.bases["first"] = 0
            self.__put_out()
    elif base == "third":
        if self.bases["second"] != 0 and self.bases["third"] == 0:
            self.bases["third"] = 0
            self.bases["second"] = 0
            self.__put_out()
    else:
        if self.bases["third"] != 0:
            self.bases["third"] = 0
            self.__put_out()

def picked_off(self, base):
    '''Runner picked off from a base.

    base - the base that the runner was on 
    (first, second, third)

    If the base is empty, an error is
    assumed to have occured, and nothing will happen.

    '''
    if self.bases[base] != 0:
        self.bases[base] = 0
        self.__put_out()

def ground_out(self):
    '''Batter grounds out.'''
    self.__next_batter()
    if self.outs == 2:
        self.__side_retired()
    else:
        self.__put_out()

def fly_out(self):
    '''Batter flies out.'''
    self.__next_batter()
    if self.outs == 2:
        self.__side_retired()
    else:
        self.__put_out()

def balk(self):
    '''Pitcher performs a balk.'''
    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

def base_hit(self, hit_type="single", 
             first_to_third=False,
             first_to_home=False,
             second_to_home=False):
    '''Batter gets a base hit.

    By default, any runners on base will advance by the 
    number of bases that were hit for.

    first_to_third, first_to_home and second_to_home can
    be set to true if a baserunner was to advance more 
    bases than the hit.  They only have meaning in the case
    of a single or double, as all runners would score on a
    triple.

    '''
    if hit_type == "single":
        self.__single()
        if second_to_home:
            self.__advance_runner_from("third")
        if first_to_third:
            self.__advance_runner_from("second")
        elif first_to_home:
            self.__advance_runner_from("second")
            self.__advance_runner_from("third")
    if hit_type == "double":
        self.__double()
        if first_to_home:
            self.__advance_runner_from("third")
    if hit_type == "triple":
        self.__triple()
    if hit_type == "homer":
        self.__homer()
    self.__next_batter()

def double_play(self, base1="second", base2="first"):
    '''Batter hits into a double play.

    base1 - base where first out is made, assume force out.
    base2 - base where second out is made.

    '''
    if self.__men_on() == 0:
        # Maybe log a warning here, or throw exception
        return
    self.__next_batter()
    if self.outs > 0:
        self.__side_retired()
    else:
        self.outs += 2
        self.bases[base1] = 0
        self.bases[base2] = 0

def triple_play(self):
    '''Batter hits into a triple play.'''
    if self.__men_on() < 2:
        # Maybe log a warning here, or throw exception
        return
    self.__next_batter()
    self.__side_retired()

def fielders_choice(self, base="second"):
    '''Batter hits into a fielder's choice.

    A fielder's choice is when a runner is on base, the
    batter hits the ball such that the runner is put out,
    but the batter is then able to get on base.

    base - base where the out is made, assume force out.

    '''
    self.__next_batter()
    if self.outs == 2:
        self.__side_retired()
    else:
        self.outs += 1
        self.bases[base] = 0

def error(self, advance_from=None):
    '''Fielder makes an error.  By default nothing happens
    other than to take note of the error.

    advance_from is a list of bases to advance runners from.  
    The base will be cleared, and the runner moved to the next
    base.  Advancing from third base will score the runner.

    If a string is used for advance_from, I just put it in a
    list.

    '''
    if isinstance(advance_from, str):
        advance_from = [advance_from]
    if advance_from:
        for base in advance_from:
            if self.__empty_base(base):
                # Maybe log a warning here, or throw exception
                return
    if advance_from:
        if "third" in advance_from:
            self.__advance_runner_from("third")
        if "second" in advance_from:
            self.__advance_runner_from("second")
        if "first" in advance_from:
            self.__advance_runner_from("first")
        if "home" in advance_from:
            self.__advance_runner_from("home")

def sacrifice(self, advance_from=None):
    '''Batter performs a sacrifice bunt or fly out.  By 
    default all baserunners advance one base.

    advance_from is a list of bases to advance runners from.  
    The base will be cleared, and the runner moved to the next
    base.  Advancing from third base will score the runner.

    If a string is used for advance_from, I just put it in a
    list.

    '''
    if isinstance(advance_from, str):
        advance_from = [advance_from]
    if self.__men_on() == 0:
        # Maybe log a warning here, or throw exception
        return
    if advance_from:
        for base in advance_from:
            if self.__empty_base(base):
                # Maybe log a warning here, or throw exception
                return
    self.__next_batter()
    if self.outs == 2:
        # Again, this can't be a sac with 2 outs.
        # Throw exception
        self.__side_retired()
    else:
        self.outs += 1
    if advance_from == None:
        self.__advance_runner_from("third")
        self.__advance_runner_from("second")
        self.__advance_runner_from("first")
    else:
        if "third" in advance_from:
            self.__advance_runner_from("third")
        if "second" in advance_from:
            self.__advance_runner_from("second")
        if "first" in advance_from:
            self.__advance_runner_from("first")

def __empty_base(self, base):
    '''Tests if a base is empty.  Returns True if the base
    is empty, false otherwise.   Special case for home.

    '''
    if base == "home":
        return False
    if self.bases[base] == 0:
        return True
    return False

def __advance_runner_from(self, base):
    '''Advance a runner from the base to the next one, and
    clear the base behind.  Advancing from third base will
    score the runner.

    Advancing from home infers that the batter got on base
    through an error.

    '''
    if base == "third":
        if self.bases["third"] != 0:
            self.bases["third"] = 0
            self.__score_run()
    if base == "second":
        if self.bases["second"] != 0:
            self.bases["second"] = 0
            self.bases["third"] = 1
    if base == "first":
        if self.bases["first"] != 0:
            self.bases["first"] = 0
            self.bases["second"] = 1
    if base == "home":
        self.bases["first"] = 1
        self.__next_batter()

def __new_inning(self):
    '''Start a fresh inning.'''
    self.bases = {"first": 0, "second": 0, "third": 0}
    self.count = {"balls": 0, "strikes": 0}
    self.outs = 0
def __men_on(self):
    '''Return a count of how many base runners there are.'''
    men_on = 0
    for base in self.bases:
        if self.bases[base] != 0:
            men_on += 1
    return men_on

def __side_retired(self):
    '''Three outs, time to switch sides, or end the game.'''
    if self.inning["val"] >= 9:
        if self.inning["half"] == "bottom":
            if self.score["home"] != self.score["away"]:
                self.__end_game()
        else: # top
            if self.score["home"] > self.score["away"]:
                self.__end_game()
    if self.inning["half"] == "top":
        self.inning["half"] = "bottom"
    else:
        self.inning["half"] = "top"
        self.inning["val"] += 1    
    self.__new_inning()

def __end_game(self):
    '''Game is over, winner is declared.

    When would we ever have a tie?

    '''
    if self.score["home"] > self.score["away"]:
        self.winner = "home"
    else:
        self.winner = "away"
    self.game_over = True
    return

def __strike_out(self):
    '''Batter strikes out.'''
    self.__next_batter()
    if self.outs == 2:
        self.__side_retired()
    else:
        self.outs += 1

def __put_out(self):
    '''Runner is put out.'''
    if self.outs == 2:
        self.__side_retired()
    else:
        self.outs += 1

def __next_batter(self):
    '''Next batter is up in the lineup.'''
    if self.inning["half"] == "top":
        if self.away_batter == 9:
            self.away_batter = 1
        else:
            self.away_batter += 1
    else: # bottom
        if self.home_batter == 9:
            self.home_batter = 1
        else:
            self.home_batter += 1
    self.count["balls"] = 0
    self.count["strikes"] = 0

def __score_run(self):
    '''Score a run.'''
    if self.inning["half"] == "bottom":
        self.score["home"] += 1
    else:
        self.score["away"] += 1

def __walk(self):
    "Walk the batter."
    if self.bases["first"] != 0:
        if self.bases["second"] != 0:
            if self.bases["third"] != 0:
                self.__score_run()
            else:
                self.bases["third"] = 1
        else:
            self.bases["second"] = 1
    else:
        self.bases["first"] = 1
    self.__next_batter()

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"] = 1

def __triple(self):
    '''Batter hits a triple.'''
    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.__score_run()
        self.bases["first"] = 0
    self.bases["third"] = 1

def __homer(self):
    '''Batter hits a home run.'''
    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.__score_run()
        self.bases["first"] = 0
    self.__score_run()

The test module wouldn't fit, so you'll have to see it on bitbucket.

share|improve this question
You could probably treat bases as a list rather than a dictionary. Then __homer() could simply do self.score["home/away"] = sum(self.bases) and then self.bases = [0, 0, 0] – Joel Cornett May 3 '12 at 3:29

1 Answer

# 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 = 1

You'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 = 0

This 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 += 1

The 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"] = 1

These 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)
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.