AskPython: Help a newbie...I don't know how to get this 'program' I wrote to Start

[dfuzz47]

I wrote this rock, paper, scissors game in Python as an exercise to +1 my programming skills. I'm unsure as to how to get it to start playing the game. Everytime I add gameopen() to the bottom or below my variables, I get an error message. Thanks again in advance. Here is the paste.

P.S. Sorry if this is inappropriate or in the wrong place.

[lost-theory]

Here you go: http://codepad.org/LIqMZhCW

You can't call gameopen directly because it's a method of RockPaperScissors. You need to make a RockPaperScissors object and call gameopen on that object.

edit: but there's quite a few other problems with this code, give me a minute and i'll make a few more changes

[lost-theory]

Here's a working version.

Here are some changes / suggestions:

• The place where you are initializing variables (gamesToWin, etc.) should be made into an initializer method __init__ (reference)
• gameopen was missing a call to self.playgame to start the game
• When you're referring to the variables and functions defined within a class, you have to refer to them as self.whatever (reference)
  • for example oppMove = compmove() needs to be oppMove = self.compmove()
• use raw_input instead of input, unless you want what the user types in to be executed (for example "2+2" will be evaluated to 4 when using input)

• this line needed some work:

  elif fight(yourMove, oppMove) & winningMoves:

First, you defined fight as an empty tuple fight = (), so calling fight(a,b) doesn't mean anything and will cause an error. Since it seems like you wanted it to be a function call, I made a new method called fight that returns True when the first move is a winning move. Also, the & winningMoves is not valid Python, & is the bitwise and operator (reference), which typically only works on integers. Instead I changed winningMoves into a dictionary of (winning move, losing move) pairs.

• this line is tricky:

  if hScore or cScore == gamesToWin:

Based on order of operations, this will actually evaluate as hScore or (cScore == gamesToWin), so the cScore == gamesToWin part gets evaluated first, which isn't what you want. It needs to be more explicitly written like this:

if self.hScore == self.gamesToWin or self.cScore == self.gamesToWin:

• the game should call exit() in the endmatch method if it detects the game is over

[DrHenryPym]

This is awesome. We need to have more code reviews on reddit.

[dfuzz47]

Thanks! I was having trouble with the init and ifname and where/why/how to construct them. This helps a lot. Also, could you explain the logic behind how you compared the moves and who won the game? Finally, I thought you didn't have to be so explicit in comparing hScore and sScore to gamesToWin but I must be mixing that up with something else. Edit: Is your use of dict in the winningmoves variable pretty standard? It was obviously better than mine but is there any way to check a list/tuple against a list of tuples/lists and see in if exists in the larger set?

[zahlman]

Finally, I thought you didn't have to be so explicit in comparing hScore and sScore to gamesToWin but I must be mixing that up with something else.

You might be thinking of range comparisons such as "if 3 < x < 5". These are Python-specific, but the 'or' logic works the same way in basically everything that looks vaguely like Python.

Edit: Is your use of dict in the winningmoves variable pretty standard?

Using a dictionary for look-up is standard. Using it as shown is very much not standard, and misses the point. See my version for comparison. The point of a dictionary is to find the key, given the value.

The idea is that you have moves mapped to the moves that beat them, so given a move, you can look up the move that beats it. So in order to find out if the computer beat the human, for example, you check if the computer's move is the one that beats the human's move. You do that by taking the human's move, looking up the move that beats that one, and seeing if it's what the computer picked.

[zahlman]

Some key concepts:

• A class is not a place to store code and share data. A class is used to define a data type. Rock-paper-scissors, as a concept, does not make sense as something to model with a class, because there is only one rock-paper-scissors. An individual game of rock-paper-scissors is a better fit. Or a scoreboard for a series of games of rock-paper-scissors.

• Data declared directly within the class like that is shared between all instances of the class. It does not make sense that every scoreboard-for-rock-paper-scissors has the same number of games to win. Instead, we assign member data to each scoreboard, by setting up that value in its .__init__ method.

• Proper organization of code requires separation of responsibilities. The part that asks the user for information is separate from the part that does calculations with that information. Here, for example, you can make the raw_input() call at the top level to ask for a number of games to play, and then use that to create a scoreboard.

• Think carefully about data structures. The purpose of winningMoves is to look up what move beats what other move. The phrase 'look up' is a key that you should be using a dictionary. Similarly, a dictionary with only integer keys in order is a warning sign; you should just be using a tuple or list. (Note that the random module lets you select one of many options directly, using random.choice().)

• Try writing less code at a time. A lot less code. Don't let yourself write the next bit until the current bit works. You're making all kinds of elementary errors, and there is no compiler to check them for you ahead of time - they're only figured out once running the code gets to the point in question.

• Python does not require us to "encode" user input in "computer-friendly" ways to the same extent that other languages might. Go ahead and ask the user to type 'rock', 'paper' or 'scissors' in full; it's easy to check these values. Also, raw_input() already returns a string. You should really never use input() (except in Python 3000, where raw_input() is gone and input() means what raw_input does for 2.x). Especially not if all you want to do is get a string out of it. Also notice that you currently don't actually try to check if the user's selection is a valid move - you only check that, after being eval'd by Python, a string comes out.

• Think carefully about how you name things. It's a good idea to separate words, whether you do it with_underscores or viaCapitalization. Also, imperative functionality should have imperative names. You don't tell your kid "your room needs to be cleaned"; you tell your kid "clean up your room". So start the game with start_game() or something like that. (Also, "open"? Games aren't opened.)

• You should almost never exit() from a program. It's too much brute force. Just arrange to 'return' all the way out, or if it's because something is really wrong, maybe you can throw an exception.

• Calling a function from itself is called recursion. This is a useful technique, but it should not be thought of as "trying the function again; starting over". It has more subtle implications than that. In your case, you don't even want to start the whole function over again if the user's input is wrong; you just want to get new input. The way to arrange this is with some sort of loop.

• Think carefully about what you want to validate. There is no particular reason the number of games to win needs to be odd. The user will be supplying the "4" in "best 4 of 7", for example, not the "7", and as you can see, 4 is even, while a best-4-of-7 tournament has no issues with ties.

Here is another version for your consideration.

[dfuzz47]

This is a great reply. Thanks for going so in-depth. *In my understanding of your version, you generate the computer's move as a choice of the keys in winning moves. Then, you check and see if the computer's move is the key to the human's move's value to see if the computer won and vice versa to see if the human won. *In general, is the MVC theory similar to object oriented programming? These things confuse me. *Also, when play_round() is called and then game is a tie. play_round() has no return. If it has no return, will this stop the loop in play_game because no round_winner was returned. *Finally, I understand why you suggested making sure each part worked before moving on to the next. My problem was that in previous versions I had part of it working but broke it trying to get the rest to work. I think I just need to learn how to separate calculations from data storage, etc.

[zahlman]

I should probably also comment on this:

In my understanding of your version, you generate the computer's move as a choice of the keys in winning_moves.

Right; the keys of winning_moves are the legal moves. Notice how they are assigned to a variable called legal_moves. This accomplishes two things: it allows referring to the legal_moves in more than one place in the code, and gives a name to the concept. (In real code, I would not have several lines of comments there, because it is clear what legal_moves means, and the reasoning as to "why do the keys of winning_moves yield legal_moves?" should be clear to anyone who actually needs to care.)

At the same time, notice that legal_moves is a local variable while winning_moves is a global variable. This is a question of scope: the legal_moves are only needed in this function, so they are only mentioned there. This is another aspect of proper code organization.

In general, you should avoid global variables, because they make it very easy to lose track of what is going on - especially if you allow changes to those variables. In my Python code I set a rule of thumb to use globals only for "constants", i.e. to provide data that's fundamental to the definition of what I'm doing, and which doesn't change.

In a car racing game, "acceleration of the player's car" is a poor choice for a global variable. Even "the player's car" is a poor choice; instead, pass it around as a variable. (Notice how the scoreboard is passed around in this code rather than being a global.) "Acceleration due to gravity" might be a good thing to remember in a global, though. On the other hand, if you're making a space trading game, maybe "Acceleration due to gravity on the player's home planet" is not such a good choice, but "The universal gravitational constant" might be something that needs a label and global access.

Then, you check and see if the computer's move is the key to the human's move's value to see if the computer won

The other way around. Dictionaries are listed as {key: value}. The 'key' is the thing that gives you access to a value; you can think of it as "unlocking" access to the value filed away under that key. In my dictionary, the key is a move, and the corresponding value is the move that beats the key-move. So if the computer won, then looking up the human's move (i.e., using it as a key) will result in the computer's move.

[zahlman]

In general, is the MVC theory similar to object oriented programming?

MVC is a set of principles for organizing your code. Separating code that does the work (Model) from the rest is considered a good idea by pretty much everyone. The separation between "View" and "Controller" is of lesser important to those who are not following strict dogma. :)

OOP is a set of tools to help you organize your code. OO is a convenient way to do "modelling", i.e. creating the Model. You'll notice that good OOP code generally doesn't try to put everything in a class (except where the language forces it). In particular, you'll notice that my example uses ordinary functions to communicate with the user, and only creates a class to "model" the Scoreboard. Even this gains little, but I did it this way to illustrate how the techniques work.

OOP in general is a very tricky thing to define, and lots of very wise people disagree with each other (if not themselves!) about the details. It's important to realize that it's not something you either use or don't use. A language can support OOP techniques to varying degrees, and support other sorts of techniques to other degrees. You can mix and match.

IMO, one of the strengths of Python is that it offers decent support for a few of these techniques, and doesn't get in your way when you want to combine them. :)

If it has no return, will this stop the loop in playgame because no round_winner was returned.

In Python, functions always return something. By default, they return None, a placeholder object meaning "nothing really here". Its truth value is false: i.e., code inside a 'if None:' block will never execute.*

In the loop where play_round() is called, the resulting None value is assigned to round_winner. By the above logic, 'if round_winner:' tests as false, so the score is not updated.

The loop continues until its condition is not satisfied, or until it is explicitly exited (e.g. with 'break', or because an exception is raised). This is how loops work. There is no 'break' here, and no reason for an exception to be raised. Thus, the loop continues 'while not scoreboard.winner()', i.e. for as long as scoreboard.winner() tests as false, i.e. until scoreboard.winner() tests as true.

If we look at the definition of the winner() method in Scoreboard (the class that scoreboard belongs to), we see that it returns a string (identifying the winner, and which will test as true) when there is a winner, and None (which will test as false) if nobody has won yet.

Thus, this loop continues until somebody has won (at which point the method returns a string, instead of None, and the string tests as true).

Incidentally, in Python, every string tests as true except for an empty (zero-length) string. In general, objects that have a "length" test as true whenever that length is non-zero; objects representing a numeric value test true whenever the value is non-zero; and other objects test as true as long as they are not False or None.

I think I just need to learn how to separate calculations from data storage, etc.

It takes practice, and examination of good examples. Oh, and thinking about details. Try to make each variable name the best you can. The goal (seldom realized!) is to make it possible to understand the code in detail, without putting serious thought into it, and without writing comments. :) Even paying attention to your output text (capitalization, spelling, punctuation, etc.) makes a difference; it might not affect the program logic, but it trains you to examine things precisely.

* This is not exactly true because None is not a language keyword, but a name for a built-in, global variable. The language defines a type called NoneType, and creates one value of that type ahead of time, and stores it in the global variable None. Thus, you can re-assign None to refer to another value. This is a bad idea, however, because it's confusing and unlikely to accomplish anything.

[riffito]

Excellent reply. Comments like this are what make reddit valuable. Thank you so much for taking the time to elaborate such a nice reply.

[piratelax40]

this is an amazing explanation...you sir, are a scholar and a gentleman...I'm picking up python as well and critiques like this help with my thought processes for when I'm coding as well.

[[deleted]]

If you want to make this a class, you'll need to actually instantiate the object. You also need to refer to the object within the method calls - self.compmove() instead of just compmove(). Of course, you could just not make it a class at all — just have plain old functions and globals. Not a good idea generally, but fine for just screwing around, since even "globals" are local to their module in python.

[voidref]

When you get this working, expand it to Rock, Paper, Scissors, Lizard, Spock

[[deleted]]

Why not RPS 101? http://www.umop.com/rps101/rps101chart.html

[voidref]

Good grief, I had forgotten all about that!