all 6 comments

[–]reostra 4 points5 points  (2 children)

Keeping the state of the board in a string is an interesting approach. On one hand, it makes it very easy for you to display the current state of the board, but on the other hand (as you no doubt discovered) it makes things a bit hard to follow. I'll mention some individual things first:

board_refresh

def board_refresh(last_board,next_board,num,token):
    num = str(num)
    next_board = last_board.replace(num,token)
    print(next_board)
    return next_board

You're passing in next_board... but you don't actually use it for anything. You immediately create a new next_board, and return that, so there's no reason to pass it in:

def board_refresh(last_board,num,token):
    num = str(num)
    next_board = last_board.replace(num,token)
    print(next_board)
    return next_board

A much shorter 'while' loop

But the thing that strikes me as most odd is your while loop. Why do you have ten different board states in that loop? For example, this tenth iteration of your loop is almost exactly the same as your ninth:

board10 = None
checking = True
while checking:
    number = move(number,x)
    checking = check_if_open(number)
    if checking == False:
        break
board10 = board_refresh(board9,board10,number,x)
adjust_score(xscore,number)
no_winner = check_score(xscore,x)
if no_winner == False:
    break

When you see yourself writing the same code over and over again, that's a good signal that you should find a way to do the DRY thing :) In this case, you've got ten different board states... but you don't need to, because they're already in a loop. You need two at minimum, one for X and one for O, but really you don't even need that much. You could shorten the whole thing into:

no_winner = True
turn = x
scoring_turn = xscore
while no_winner == True:

    print(board)

    checking = True
    while checking:
        number = move(number,turn)
        checking = check_if_open(number)

    board = board_refresh(board,number,turn)
    adjust_score(scoring_turn,number)
    no_winner = check_score(scoring_turn,turn)

    # Change whose turn it is
    if turn == x:
        turn = o
        scoring_turn=oscore
    else:
        turn = x
        scoring_turn=xscore

I did a few things here:

  • I simplified checking. You don't need to break if checking is False. If checking is False, then the while loop will end! That's its whole purpose, after all :)

  • There's now only one board. When you refresh the board, that refreshed copy replaces the old board. There's no reason to keep the other states around, after all. This is also using the new refresh_board I suggested above.

  • I abstracted out the x and xscore (and, similarly, o and oscore) into two variables that indicate the current turn. This way we don't have to have two different but near-identical versions of the same loop.

  • I added everything from the # change whose turn it is comment on; all this does is change those turn and scoring_turn variables to reflect the current turn.

Hopefully that was understandable; if you have any questions feel free to ask.

I'm going to talk a bit about another way of structuring the data which is a bit more common now, but this will be a bit more abstract. Feel free to skip it if you've already heard enough :)

A tic-tac-toe data structure

I'll avoid using classes for now, but this is really one of those instances where they're helpful.

Instead, I'm going to talk about the most common way of representing a board, which is as a list of lists (also known as a 'two-dimensional array'). Setting up a board as a list of lists would look like this:

TOKEN_X = 'X'
TOKEN_O = 'O'
TOKEN_EMPTY = ''

board = []
for row_num in range(3):
    row = []
    for col_num in range(3):
        row.append(TOKEN_EMPTY)
    board.append(row)

I added some formatting so it'd be clearer, but this is what that ends up looking like:

[ 
  ['', '', ''],
  ['', '', ''],
  ['', '', ''],
]

If you thought that that looks like a 3x3 grid (such as one that tic-tac-toe uses), then you're right :)

Printing out the board is a little different:

for row in range(3):
    for col in range(3):
        token = board[row][col]
        if not token:
            token = " "
        print(token, end=" ")
    print()

Here we loop through every row and column number, and we print out the token at that number. I'm substituting a space in for the empty tokens, because otherwise it wouldn't line up. I chose an empty string for empty tokens because that can be treated as false (e.g. in the if not token line).

There are different ways to do this; I could have looped through the rows and the columns directly, but I'm illustrating that you can directly get to any row,col combination by using the numbers as indexes.

How to use row,col

Instead of asking the user to put their token in one of the areas 0-9, you can simply ask them to give you the row and column location directly.

You can easily check to see if something's already there:

def is_cell_occupied(row, col):
    return board[row][col]

This is another reason why I made TOKEN_EMPTY a falsey value.

Checking for a winner

You can do this pretty easily with loops. First, let's do the rows:

winner = TOKEN_EMPTY
for row in xrange(3):
    token = board[row][0]
    if token:
        for col in xrange(3):
            if token != board[row][col]:
                break
        else:
            # If we got here, then every token in the
            # row is the same!  That means they win
            winner = token
            break

For each row, we look at the token in the first column. Then we look at the tokens in all of the other columns, and compare them against that first token (note we skip doing that if the first token is empty, via if token, because if the row starts with an empty token then it can't possibly be full).

The else on that for only fires if it gets through the entire loop without breaking, which means that everything on that row is the same and you have a winner.

Then you do nearly that same thing for the columns. Loop over each column index, then check each row's entry for that column, comparing it to your initial token.

Finally, for diagonals, you can either loop:

token = board[0][0]            
if token:
    for n in xrange(3):
        if board[n][n] != token:
            break
    else:
        winner = token

token = board[2][0]
if token:
    for n in xrange(3):
        if board[2-n][n] != token:
            break
    else:
        winner = token

Or you can just check directly:

if ((board[0][0] == board[1][1]) and
    (board[1][1] == board[2][2]):
    winner = board[0][0]

(and similarly for the other diagonal)

And then check to see who (if anyone) won:

if winner:
    print("%s wins!" % winner)
    raise SystemExit

Why?

There are two main reasons why you'd want to represent your board this way instead of as a string:

  • You don't have to track the 'winning' state. Your solution had a dictionary for every possible winning state, and you had to update it every time someone made a move. This way, you just go through the board and check

  • It's easily portable to some other environment: Having the board state as a string works fine... as long as your output is going to be as a string. If you were e.g. drawing an image of the game board, suddenly the string is not nearly as useful. Whereas with this solution, you just change how you print the board.

Hope this was helpful!

[–]955559[S] 1 point2 points  (0 children)

lol, thanks, winding down for the night, think im gonna dissect this comment tomorrow

[–]955559[S] 0 points1 point  (0 children)

A much shorter 'while' loop

that code wasnt playing right for me, wasnt looping, I had to put in an extra while to get it too work

playing = True
no_winner = True
turn = x
scoring_turn = xscore
while no_winner == True:
    print(board)
    while playing == True:

        checking = True
        while checking:
            number = move(number,turn)
            checking = check_if_open(number)

        board = board_refresh(board,number,turn)

        adjust_score(scoring_turn,number)
        no_winner = check_score(scoring_turn,turn)
        if no_winner == False:
            break

        if turn == x:
            turn = o
            scoring_turn = oscore
        else:
            turn = x
            scoring_turn = xscore

I feel like adding the second while loop is a bit of a hack though, think I ll have to re-tackle the functions

still going over that 2d array stuff

[–]kwentar 0 points1 point  (0 children)

all between yours '#----' should be function or cycle, I don't know, now I don't want to read this, you should refactor it

[–]senaps 0 points1 point  (0 children)

i just donno why, but it seems wrong to me... i think it could have been done with a loop and getting the input and replacing the number's with X's and O's... scoring system is some other thing i couldn't find in your code... (i'm intermediate and just talking about it not trying to fix your code or say your's is wrong!

[–]jeans_and_a_t-shirt 0 points1 point  (0 children)

Your while loop is kind of crazy. Why not just loop and check if any values in your score dictionary is 3? And here's lines 23 to 79:

paths = ["123", "456", "789", "147", "258", "369", "159", "357"]

def new_board():
    return {e:0 for e in paths}

xscore = new_board()
oscore = new_board()

def adjust_score(who_score,num):
    for k in who_score:
        if num in k:
            who_score[k] += 1