3
\$\begingroup\$

I am trying to learn Python 3.6 with the help of a book. Until now I have learned the basics of functions, conditions and printing etc. No loops yet - so they need to come from recursion.

To test the current stuff I wrote this basic TicTacToe - and would like to know what could be better.

def print_game(*args):
    formatstring = "   A B C\n" + "-" *10 + "\n1  {}|{}|{}\n" + "-" *10 + "\n2  {}|{}|{}\n" + "-" *10 + "\n3  {}|{}|{}"
    print(formatstring.format(*args))

def ask_player(round):
    prompt_player1 = "Player one>> "
    prompt_player2 = "Player two>> "

    if round % 2 == 0:
        current_choice = input(prompt_player2)
        current_player = 2
    else:
        current_choice = input(prompt_player1)
        current_player = 1
    if current_choice != "A1" and current_choice != "B1" and current_choice != "C1"  and current_choice != "A2" and current_choice != "B2" and current_choice != "C2" and current_choice != "A3" and current_choice != "B3" and current_choice != "C3":
        print("Please use Field identifier like A1/B3 ...")
        ask_player(round)
    else:
        return current_choice, current_player

def do_the_draw(game,field,player):
    valid = 1
    if player == 1:
        char='X'
    if player == 2:
        char='O'
    if field == 'A1' and game[0]==' ':
        game[0] = char
    elif field == 'B1' and game[1]==' ':
        game[1] = char
    elif field == 'C1' and game[2]==' ':
        game[2] = char
    elif field == 'A2' and game[3]==' ':
        game[3] = char
    elif field == 'B2' and game[4]==' ':
        game[4] = char
    elif field == 'C2' and game[5]==' ':
        game[5] = char
    elif field == 'A3' and game[6]==' ':
        game[6] = char
    elif field == 'B3' and game[7]==' ':
        game[7] = char
    elif field == 'C3' and game[8]==' ':
        game[8] = char
    else:
        print(f"Invalid Move field {field} is already used - Choose another one")
        valid = 0
    print_game(*game)
    return game,valid

def evaluate(s):
    result = False #game continues
    # Horizontal lines
    if s[0]+s[1]+s[2] == 'XXX' or s[0]+s[1]+s[2] == 'OOO':
        result = True
    if s[3]+s[4]+s[5] == 'XXX' or s[3]+s[4]+s[5] == 'OOO':
        result = True
    if s[6]+s[7]+s[8] == 'XXX' or s[6]+s[7]+s[8] == 'OOO' :
        result = True
    # Vertical lines
    if s[0]+s[3]+s[6] == 'XXX' or s[0]+s[3]+s[6] == 'OOO':
        result = True
    if s[1]+s[4]+s[7] == 'XXX' or s[1]+s[4]+s[7] == 'OOO':
        result = True
    if s[2]+s[5]+s[8] == 'XXX' or s[2]+s[5]+s[8] == 'OOO' :
        result = True
    # Diagonal lines
    if s[0]+s[4]+s[8] == 'XXX' or s[1]+s[4]+s[7] == 'OOO':
        result = True
    if s[2]+s[4]+s[6] == 'XXX' or s[2]+s[5]+s[8] == 'OOO' :
        result = True
    return result


def main(draws,status):
    user_input, player = ask_player(draws)
    status,valid = do_the_draw(status,user_input,player)
    if valid == 0:
        return main(draws,status)
    draws -=1
    if evaluate(status):
        draws = False
        print(f"We have a winner Player: {player}")
    if draws:
        return main(draws,status)
    elif input("try again?")=='y':
        print("#" *100)
        print("Here we go again\n\n")
        draws = 9
        status = " , , , , , , , , "
        status = status.split(',')
        print_game(*status)
        return main(draws,status)



# Print the initial Game pattern
themaingame=" , , , , , , , , "
# cause i was not able to figure out how to initialise lists ... we split
themaingame = themaingame.split(',')
# Initial Print of the Board
print_game(*themaingame)

#Calling the Main Method with the maximum number of draws (9) and the prepared List
main(9,themaingame)
\$\endgroup\$
4
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Ludisposed
    Jan 8, 2019 at 12:32
  • \$\begingroup\$ I have rolled back your last edit. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Heslacher
    Jan 8, 2019 at 12:48
  • \$\begingroup\$ Isn't this a duplicate to codereview.stackexchange.com/questions/211067/… \$\endgroup\$
    – instaggy
    Jan 8, 2019 at 12:59
  • \$\begingroup\$ thanks for pointing out - I was not sure how to do it and I only found the code of conduct. \$\endgroup\$
    – Patrick
    Jan 8, 2019 at 13:24

1 Answer 1

3
\$\begingroup\$

maybe I have some useful suggestions.

I think having main recursively call itself is a little weird. Try to keep the main very short, so my suggestion create a function called play_tictactoe() and call that from main instead. This also means play_tictactoe recursively calls itself instead of main.

You have a function called do_the_draw() but it also updates the game board. Functions typically do 1 thing and it should be obvious based on it's name. You could separate your draw logic and update logic. You could just rename do_the_draw() to update_board() and remove the drawing logic and call it in the play function.

evaluate can be simplified by supplying a symbol. evaluate(s, char) could reduce the function by half, so it could be evaluate(status, 'OOO') and evaluate(status, 'XXX') which will result in less typing errors like you have for if s[0]+s[4]+s[8] == 'XXX' or s[1]+s[4]+s[7] == 'OOO':

Finally if you have access to maps or arrays (ignore this if you do not) you can probably reduce some of those huge if groups down using a map. I won't go into too much detail since you probably don't have access but a map of field to numbers such as { "A1": 0, "A2": 1 } could help reduce the long if statement by just checking if the map has a key and the other if statement since it could just check game[fields[field]] == ' '. If you have access to array functions you could do something similar with just arrays and using their built in functions so you do not need to loop over them yourself.

Hopefully that makes sense and it was an inter

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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