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

Python: Making bowling algorithm more pythonic

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

Problem

So I created an algorithm that keeps score of the amount of pins knocked down per frame. Since I am totally self taught, I am trying to see how I could make my code more efficient/more pythonic. Any advice would help. Also, is it proper to return a score or list into the init method as I did with the "overall" variable? Thanks.

class Bowling_player(object):

    def __init__(self, name, overall = 0):
        self.name = name
        self.overall = overall

    def frames(self):
        shot_one = int(input('how many pins on first shot:'))
        if shot_one == 10: # this is a strike
            shot_two_after_strike = int(input('how many pins on shot after strike:'))
            if shot_two_after_strike == 10: # you got another strike. continue the frame!
                shot_three_after_strike = int(input('how many pins on shot after two strikes:'))
                self.overall += (shot_three_after_strike + shot_two_after_strike + shot_one)
                return self.overall
            else:# you did not get another strike, frame over.
                self.overall += (shot_one + shot_two_after_strike)
                return self.overall

        elif shot_one < 10:
            shot_two = int(input('how many pins on second shot:'))
            shots = shot_one + shot_two
            if shots == 10: # this is a spare          
                shot_three_after_spare = int(input('how many pins on shot after spare'))  
                self.overall += (shots + shot_three_after_spare)
                return self.overall
            else: # you suck at bowling 
                self.overall += shots
                return self.overall

craig = Bowling_player('craig')

craig.frames()

Solution

Overall, there is not much sense in using a class here - you are not using player's name and you are calculating the overall shots in the frames method and returning the overall value.

Some code style specific notes:

  • according to PEP8, the class name should be in CamelCase, call it BowlingPlayer



-
there is no need for parenthesis around the expressions, you can replace:

self.overall += (shot_three_after_strike + shot_two_after_strike + shot_one)


with:

self.overall += shot_three_after_strike + shot_two_after_strike + shot_one


-
there is some code duplication when you calculate the overall value, you can add up shot_two_after_strike and shot_one to the overall value and only if the score is 10, add the shot_three_after_strike:

self.overall += shot_two_after_strike + shot_one
if shot_two_after_strike == 10: # you got another strike. continue the frame!
    shot_three_after_strike = int(input('how many pins on shot after two strikes:'))
    self.overall += shot_three_after_strike
return self.overall


Same for the case when shot_one

-
according to PEP8 commenting guidelines, there has to be two spaces before the
# and once space after

  • there is no need for spaces around the = for keyword function or method arguments (PEP8 reference)



  • put the main execution code to under if __name__ == '__main__': to avoid it being executed on import



  • let's define 10 as a constant to avoid hardcoding things



  • I'd also use something like a total_points variable name instead of a more questionable overall`



Improved code:

POINTS_FOR_STRIKE = 10

class BowlingPlayer(object):
    def __init__(self, name, overall=0):
        self.name = name
        self.total_points = overall

    def frames(self):
        shot_one = int(input('how many pins on first shot:'))
        if shot_one == POINTS_FOR_STRIKE:  # this is a strike
            shot_two_after_strike = int(input('how many pins on shot after strike:'))
            self.total_points += shot_two_after_strike + shot_one

            if shot_two_after_strike == POINTS_FOR_STRIKE:  # you got another strike. continue the frame!
                shot_three_after_strike = int(input('how many pins on shot after two strikes:'))
                self.total_points += shot_three_after_strike
        elif shot_one < POINTS_FOR_STRIKE:
            shot_two = int(input('how many pins on second shot:'))
            shots = shot_one + shot_two
            self.total_points += shots

            if shots == POINTS_FOR_STRIKE:  # this is a spare
                shot_three_after_spare = int(input('how many pins on shot after spare'))  
                self.total_points += shot_three_after_spare
        return self.total_points

if __name__ == '__main__':
    craig = BowlingPlayer('craig')
    print(craig.frames())

Code Snippets

self.overall += (shot_three_after_strike + shot_two_after_strike + shot_one)
self.overall += shot_three_after_strike + shot_two_after_strike + shot_one
self.overall += shot_two_after_strike + shot_one
if shot_two_after_strike == 10: # you got another strike. continue the frame!
    shot_three_after_strike = int(input('how many pins on shot after two strikes:'))
    self.overall += shot_three_after_strike
return self.overall
POINTS_FOR_STRIKE = 10


class BowlingPlayer(object):
    def __init__(self, name, overall=0):
        self.name = name
        self.total_points = overall

    def frames(self):
        shot_one = int(input('how many pins on first shot:'))
        if shot_one == POINTS_FOR_STRIKE:  # this is a strike
            shot_two_after_strike = int(input('how many pins on shot after strike:'))
            self.total_points += shot_two_after_strike + shot_one

            if shot_two_after_strike == POINTS_FOR_STRIKE:  # you got another strike. continue the frame!
                shot_three_after_strike = int(input('how many pins on shot after two strikes:'))
                self.total_points += shot_three_after_strike
        elif shot_one < POINTS_FOR_STRIKE:
            shot_two = int(input('how many pins on second shot:'))
            shots = shot_one + shot_two
            self.total_points += shots

            if shots == POINTS_FOR_STRIKE:  # this is a spare
                shot_three_after_spare = int(input('how many pins on shot after spare'))  
                self.total_points += shot_three_after_spare
        return self.total_points


if __name__ == '__main__':
    craig = BowlingPlayer('craig')
    print(craig.frames())

Context

StackExchange Code Review Q#159135, answer score: 3

Revisions (0)

No revisions yet.