Rock Paper Scissors with scores and save file

Question

I've implemented the classic game Rock Paper Scissors with scores, and a save file implementation. I'm looking particularly for ways to get rid of all the self.'s. Is there anything I should add, remove, or change. Are there any bugs? Anything else?

#Programmer: 
import random as ran
import shelve

class Score(object):
    """Place where all scores are stored for save document"""

    def __init__(self):
        """Makes all variables set to 0"""
        self.rounds = 0
        self.losses = 0
        self.wins = 0
        self.draws = 0
        self.game_wins = 0
        self.game_losses = 0
        self.total_rounds = 0
        self.total_games = 0
        self.round_wins = 0
        self.round_losses = 0
        self.round_draws = 0

    def reset(self):
        """Resets the scores for next game"""
        self.wins = 0
        self.draws = 0
        self.losses = 0

    def tally(self):
        """Adds the score to stats"""
        self.total_rounds += self.rounds
        self.total_games += 1
        self.round_wins += self.wins
        self.round_losses += self.losses
        self.round_draws += self.draws

    def stats(self):
        """Prints the stats"""
        print ('\n\n+++++++++++++++++++++-=Stats=-+++++++++++++++++++')
        print ('=================================================')
        print ('|--           --|--   Rounds  --|--   Games   --|')
        print ('|--   Wins    --|--     %s     --|--     %s     --|')    %(self.round_wins, self.game_wins)
        print ('|--   Losses  --|--     %s     --|--     %s     --|')    %(self.round_losses, self.game_losses)
        print ('|--   Draws   --|--     %s     --|--    N/A    --|')    %(self.round_draws)
        print ('|--   Played  --|--     %s     --|--     %s     --|\n\n')%(self.total_rounds, self.total_games)

    def final(self):
        """Prints final score of the game just played"""
        print ('\n\n+++++++++++Final Score++++++++++')
        print ('================================')
        print ('|--   Wins    --|--    %s     --|')    %(self.wins)
        print ('|--   Losses  --|--    %s     --|')    %(self.losses)
        print ('|--   Draws   --|--    %s     --|')    %(self.draws)
        print ('|--   Rounds  --|--    %s     --|\n\n')    %(self.rounds)


def custom_input(question, choices):
    """A custom loop that checks to see if choices is valid"""
    response = raw_input(question).lower()
    while response not in choices:
        print ("Correct inputs: ")
        for c in choices:
            print(c)
        response = raw_input(question).lower()
    return response


class Game(object):
    """Main game"""
    def __init__(self):
        """Starts the program here."""
        try:                                    
            f = shelve.open("RPS_save.dat")     
            statistics = f["statistics"]        
            self.score = statistics[0]          
            f.close()
        except:
            self.score = Score()
            f = shelve.open("RPS_save.dat")
            statistics = [self.score]
            f["statistics"] = statistics
            f.sync()
            f.close()
        self.Continue = None
        self.game = None
        self.plays = ['rock', 'paper', 'scissors']
        self.games = ['1', '3', '5']
        self.modes = ['pvc', 'cvc']
        self.p1win = [('rock' + 'scissors'), ('paper' + 'rock'), ('scissors' + 'paper')]
        self.menu = [('stats'), ('play'), ('quit')]
        while self.Continue is None:
            self.Continue = raw_input("Welcome to the greatest, mind blowing, challenge of all "
                    "time.\n   - Rock, Paper, Scissors! \nMany have tried and many have"
                    " FAILED... \nThis will be a test between the human mind and"
                    " my AI.\nPress \"enter\" when you believe your ready for this "
                    "challenge.\n")
        print "Good Luck... Human."
        while True:
            self.game = custom_input("Would you like to play, look at stats, or quit?\n", self.menu)
            if self.game == "play":
                self.rounds = custom_input("How many rounds would you like? \n", self.games)
                self.play_game()
                self.score.tally()
                self.score.final()
                self.score.reset()
                f = shelve.open("RPS_save.dat")
                statistics = [self.score]
                f["statistics"] = statistics
                f.sync()
                f.close()
            elif self.game == "stats":
                self.score.stats()
            if self.game == "quit":
                break


    def play_game(self):
        """This is where most of the game takes place"""
        while self.score.rounds != int(self.rounds):
            self.score.rounds += 1
            self.user_choice = custom_input("What is your choice, human! \n  ",self.plays)
            self.computer_choice = self.computer_choice_gen()
            print "\nComputer choice is %s"    %(self.computer_choice)
            result = self.evaluate()
            if result == "win":
                print "%s beats %s! The human wins this round.\n\n"   %(self.user_choice.capitalize(), self.computer_choice)
                self.score.wins += 1
            elif result == "loss":
                print "%s beats %s! Hahaha! You lost this round!\n\n" %(self.computer_choice.capitalize(), self.user_choice)
                self.score.losses += 1
            else:
                print "I knew you were going to pick %s!\n\n"        %(self.user_choice.capitalize())
                self.score.draws += 1
        self.finals()

    def finals(self):
        """Adds win /losses to scores"""
        if self.score.wins > self.score.losses:
            self.score.game_wins += 1
            print "Looks like humans are still dominant in this time. You won the Game!"
        else:
            self.score.game_losses += 1
            print "Humans are no match for my AI."

    def computer_choice_gen(self):
        """Generates computer choice"""
        return ran.choice(self.plays)

    ##def computer_choice():
    ##    chance = ran.randint(0, 99)
    ##    if chance > 66:
    ##        return "rock"
    ##    elif chance < 33:
    ##        return "paper"
    ##    else:
    ##        return "scissors"

    def evaluate(self):
        """Determines whether game is a win loss or draw"""
        if self.user_choice + self.computer_choice in self.p1win:
            return "win"
        elif self.user_choice == self.computer_choice:
            return "draw"
        else:
            return "loss"


Game()

Answer 1

Don't import random as ran! What does this save you? 3 characters, and causes headaches for anyone who misses or forgets that you've done this. You only even call it once, so aliasing it takes more characters since you need to add as ran just to avoid writing dom once.

Score is actually a pretty good way of arranging a class, you have some useful methods for it. Though a name like Scoreboard or Scorecard might be more appropriate to indicate it holds a series of scores. Also, given the nature of your class it could be easier to have it inherit from dict. That way all your attributes can be keys instead. This makes it easier to loop over setting keys and allows you to access functions like .keys() and .values(). Take a look at how your __init__ can look with this:

class Score(dict):
    """Place where all scores are stored for save document"""

    def __init__(self):
        """Makes all variables set to 0"""
        for key in ("rounds", "losses", "wins", "draws", "game_wins",
                    "game_losses", "total_rounds", "total_games",
                    "round_wins", "round_losses", "round_draws"):
            self[key] = 0

You can do a similar thing with reset too. Of course this affects how you access self.scores, instead of dot syntax you'll access it with a key. ie. self.score['rounds'] instead of self.score.rounds.

Instead of using the % syntax for string formatting you should use str.format as that's the new method. It also makes tables easier because you can specify the spacing and get messages to be centered with the syntax it allows. For example this line:

    print ('|--   Wins    --|--     %s     --|--     %s     --|')    %(self.round_wins, self.game_wins)

could be rewritten as:

    print ('|--{:^11}--|--{:^12}--|--{:^12}--|').format("Wins", self.round_wins, self.game_wins)
    # |--   Wins    --|--     6      --|--     12     --|

To explain, the values are substituted in wherever there's {} brackets. The colon separates an index value(which we haven't used) from formatting info. The ^ indicates to center the text and then the number tells Python how many spaces it should take up as a minimum. The beauty of this format is that you can reuse the same string to format for every line and just pass the relevant values:

def stats(self):
    """Prints the stats"""
    print ('\n\n+++++++++++++++++++++-=Stats=-+++++++++++++++++++')
    print ('=================================================')
    print ('|--{:^11}--|--{:^12}--|--{:^12}--|').format("", "Rounds", "Games")
    print ('|--{:^11}--|--{:^12}--|--{:^12}--|').format("Wins", self.round_wins, self.game_wins)
    print ('|--{:^11}--|--{:^12}--|--{:^12}--|').format("Losses", self.round_losses, self.game_losses)
    print ('|--{:^11}--|--{:^12}--|--{:^12}--|').format("Draws", self.round_draws, "N/A")
    print ('|--{:^11}--|--{:^12}--|--{:^12}--|').format("Played", self.total_rounds, self.total_games)
    print ('\n')

Also, are you aware of __str__? It's a magic method, if you were to change the name of stats to __str__ and returned the values instead of printing them then you could make this table be the string representation of your object. This is how it would need to look:

def __str__(self):
    """Table of all stats"""

    return ('\n\n+++++++++++++++++++++-=Stats=-+++++++++++++++++++\n'
            '=================================================\n'
            '|--{:^11}--|--{:^12}--|--{:^12}--|\n'.format("", "Rounds", "Games")
            '|--{:^11}--|--{:^12}--|--{:^12}--|\n'.format("Wins", self.round_wins, self.game_wins)
            '|--{:^11}--|--{:^12}--|--{:^12}--|\n'.format("Losses", self.round_losses, self.game_losses)
            '|--{:^11}--|--{:^12}--|--{:^12}--|\n'.format("Draws", self.round_draws, "N/A")
            '|--{:^11}--|--{:^12}--|--{:^12}--|\n'.format("Played", self.total_rounds, self.total_games)
            '\n\n')

The main change is returning instead of printing. But also manually adding newlines since it's not a print statement. Also you don't need to explicitly concatenate them. String literals are implicitly concatenated when placed next to each other without an operator or comma separator:

>>> "ban" "ana"
'banana'

It's good to see an input wrapper function, though I don't know why it's separated out from Game, especially when Game calls on it. You should include it. You can also print your choices easier using str.join. It will take an iterable of strings and then join all the strings contained in it, with a string separating values. ie. '\n'.join(choices) will put a newline character between each choice, which is the same as your for loop.

Ethan is correct that your __init__ does too much. It should be for initialising. Creating values and storing them in the proper form. Everything after self.menu should be in a run function.

Also, some of your attributes are constants that should be at the class level. plays, games, menu and p1win are all unchanged by __init__, so they should just be defined in Game itself. Also since they're constants they should be in UPPER_SNAKE_CASE naming format, and they should be tuples, not lists. Tuples are immutable and indicate a constant value much clearer.

class Game(object):
    """Main game"""

    PLAYS = ('rock', 'paper', 'scissors')
    GAMES = ('1', '3', '5')
    MODES = ('pvc', 'cvc')
    P1WIN = ('rock' + 'scissors', 'paper' + 'rock', 'scissors' + 'paper')
    MENU = ('stats', 'play', 'quit')

You can access these either with self.PLAYS or Game.Plays, but I think the latter is clearer about it being a class attribute. At that, I'd point out that MODES isn't implemented and you could use better names. PLAYS should be someting like MOVES. GAMES should be ROUNDS or ROUND_OPTIONS maybe?

Also I don't see the point of your Continue at all, it's used as a flag while self.Continue is None, but then raw_input immediately unsets that because any result of it will not be None? Just remove it, having the raw_input by itself is enough. Likewise, though you actually do use self.game you don't need to initialise it as none. It doesn't even need to be an attribute, as it's only used in one function.

You don't need to temporarily assign statistics either, you can do this:

        statistics = f["statistics"]        
        self.score = statistics[0]          

in one line:

        self.score = f['statistics'][0]

You could do a similar thing when writing to it later:

            f["statistics"] = [self.score]

Back to your long raw_input, it's way too long a string. Just print line by line and then wrap the final line in raw_input. All your newline characters in the middle of the string are awkward to read.

Just turn self.rounds into an int once instead of repeatedly calling it like this. There's no real reason to keep it a string after the user has entered it.

Your method of evaluating wins and losses is odd. Concatenated strings that indicate the player's victory are counter intuitive. You could instead have a dictionary of values doing a similar thing:

WIN_CONDITIONS = {
    "rock": "scissors",
    "scissors": "paper",
    "paper": "rock",
    }

This would make your test look like this:

    if Game.WIN_CONDITIONS[self.user_choice] == self.computer_choice:
        return "win"

It may be more verbose but it indicates the idea clearer and key value pairs are more understood than concatenated strings.

Answer 2

Proper error catching

Never, ever have an empty except clause like the following:

try:
    ...
except:
    ...

With an empty except clause, any error that occurs in the try block will be caught by the except clause. This includes errors that aren't intended to be caught, like a SystemError, where something goes wrong internally.

The proper way to specify what error you want to catch would be to write your except clause like this:

except ExceptionType:
    ...

---

Properly opening files

The generally accepted method for opening files in Python is to use a contest manager, as seen below. If you use a context manager, the file is closed implicitly, and you can guarantee that the file is closed properly if the program unexpectedly exits:

with open("path/to/my/file.dat", "mode") as my_file:
    ...

---

OOP Design

It feels like you're not using object-oriented-programming correctly, as your __init__ methods seem to be doing way too much. As the name implies, the magic method __init__ should only be used for class initialization, and nothing else. Right now, you're treating the __init__ method like the "main method" of your class.

The best way that I can think of would be to do two things:

1. Use a global state. I wouldn't highly recommend this, but if the program is small and trivial enough, then it's probably okay.
2. Represent items like a Scissor, or a Rock with objects.

---

Nitpicks

• Classes in Python 3.x are implicitly new-style, and don't need to explicitly inherit from object. This means they can be declared in a fashion like this:

  class MyClass:
      ...
  

• Functions like stats() or final() should probably return a string, rather than explicitly printing one.

Answer 3

First of all do read the excellent answer of SuperBiasedMan, on which I would like to make some additional notes:

• Instead of self[key] = 0, you could use setattr(self, key, 0), and keep your existing code like self.score

• Another alternative to the win conditions could be to use tuples, instead of string conditions:

  P1WIN = [('rock', 'scissors'), ('paper', 'rock') , ('scissors', 'paper')] 
  ...
  
  if (self.user_choice, self.computer_choice) in self.p1win:
      return "win"
  

Extend Score

I would change the Score class to have all the counters as internal variables, and add methods to change the score. You could then do a self.score.add_win() which would correctly update current wins, round wins and total.

This would also eliminate a bug in your current code. Within play_game you do self.score.wins += 1, and just after that in tally you add the accumulated value of wins & co in self.round_wins += self.wins. Bug! The wins accumulate really fast for winning rounds...

You could also have a method, in addition to those pretty print methods, which returned tuple's of the scores, i.e. three tuples for game, round and total, where each tuple had wins, losses and draws. This would allow code outside of score to see the score, but not manipulate besides the ordinary methods.

You could also allow for score to handle its own saving and loading, without too much fuzz.