-2
\$\begingroup\$

I'm a beginner in python so I decided to take a simple challenge and wrote tictactoe

How to write it better in future? What's wrong with my code?

from random import choice

again = ''
board = '[1][2][3]\n[4][5][6]\n[7][8][9]'
win_combination = [(1,2,3),(4,5,6),(7,8,9),(1,4,7),(2,5,8),(3,6,9),(1,5,9), 
(3,5,7)]
pleyer_numbers = []
computer_numbers = []
numbers = [x for x in range(1,10)

def replece_board(board):
    board = board.replace(str(pleyer_number),'X')
    board = board.replace(str(computer_number),'O')
    return board

def change_number():
    pleyer_numbers.append(int(pleyer_number))
    computer_numbers.append(int(computer_number))


def check_win(who):
    check = 0
    for win in win_combination:
        for number in win:
            if number in who:
                check += 1
                if check == 3:
                    return True
            else:
                continue            
        check = 0
    return False


print('Welcome in simple tic-tac-toe!\nYour enemy is computer.')      
while True:
    board = '[1][2][3]\n[4][5][6]\n[7][8][9]'
    pleyer_numbers = []
    computer_numbers = []
    numbers = [x for x in range(1,10)]
    while True:
        print(board)
        pleyer_number = input('Enter the field number: ')
        if int(pleyer_number) not in range(1,10):
            print('There is no filed with this number.')
            continue
        elif int(pleyer_number) not in numbers:
            print('This field is occupied.')
            continue    
        numbers.remove(int(pleyer_number))
        if numbers:
            computer_number = choice(numbers)
            numbers.remove(computer_number)
            print('Computer chose field: ' + str(computer_number))
        change_number()
        board = replece_board(board)
        if check_win(pleyer_numbers):
            print(board.replace(str(pleyer_number),'X'))
            print('----------YOU WON!----------')
            break
        if check_win(computer_numbers):
            print(board)
            print('----------YOU LOST!----------')
            break 
        if not numbers:
            print('\n----------TIE!----------')
            break   
    again = input('Do you want to play again? y/n ')
    while True:
        if again != 'y' and again != 'n':
            again = input('Only y/n ')
        else:
            break
    if again == 'n':
         break
    else:
        print('----------------------------\nYou started new game!')
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! Please make sure that the code in your question is actually working. It seems like there is at least a missing bracket at numbers = [x for x in range(1,10). \$\endgroup\$ – AlexV Sep 3 '19 at 12:02
  • 1
    \$\begingroup\$ Also, pleyer should be spelled player \$\endgroup\$ – Linny Sep 3 '19 at 12:08
  • 1
    \$\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 Sep 3 '19 at 13:04
6
\$\begingroup\$

Welcome to code review.

Bug:

unmatched parenthesis at line 9:

numbers = [x for x in range(1,10)

Style:

I suggest you check PEP0008 https://www.python.org/dev/peps/pep-0008/ the official Python style guide and here are a few comments:

win_combination = [(1,2,3),(4,5,6),(7,8,9),(1,4,7),(2,5,8),(3,6,9),(1,5,9),(3,5,7)]

a space should be left after commas for readability like this:

win_combination = [(1, 2, 3), (4, 5, 6), (7, 8, 9), (1, 4, 7), (2, 5, 8), (3, 6, 9), (1, 5, 9)] 

Spelling

pleyer_numbers = []
def replece_board(board):
print('There is no filed with this number.')

A code is also designed for human beings to read regardless of whether the machine does or does not care if names are not significant we would be using barcodes as variable names instead.)

Code

General remarks: your code is a bit longer than it should and disorganized and you might want to consider breaking down your code into functions with separate roles.

def replece_board(board):

replace what board? what is board?

def change_number():

what number?

def check_win(who):

what is who? a number? a string? a list? something else?

for each public function you make now and in the future you should include a docstring answering such obvious questions.

Docstrings: Python documentation strings (or docstrings) provide a convenient way of associating documentation with Python modules, functions, classes, and methods. It's specified in source code that is used, like a comment, to document a specific segment of code.

if __name__ == '__main__': 

guard to be used at the end of script to test your functions and this allows your module to be imported by other modules without running the whole script.

again = ''
board = '[1][2][3]\n[4][5][6]\n[7][8][9]'
win_combination = [(1,2,3),(4,5,6),(7,8,9),(1,4,7),(2,5,8),(3,6,9),(1,5,9), 
(3,5,7)]
pleyer_numbers = []
computer_numbers = []
numbers = [x for x in range(1,10)

Global variables: Are to be avoided as much as possible, a better approach is to enclose your variables inside their corresponding functions that use them

Catching invalid inputs since you're dealing with user input, it is very likely that someone enters a wrong value example:

Enter the field number: w

results in an error, you should control errors that would terminate your program

confirm_number = input('Enter field number: ')
while not confirm_number.isdecimal():
    print('You should enter integer numbers only!')
    confirm_number = input('Enter field number: ')

f-strings since you're using Python 3 I suppose from the print statements, f-strings are a new string formatting mechanism known as Literal String Interpolation or more commonly as F-strings (because of the leading f character preceding the string literal). ... In Python source code, an f-string is a literal string, prefixed with 'f', which contains expressions inside braces providing a way to embed expressions inside string literals, using a minimal syntax.

a statement like

print('Computer chose field: ' + str(computer_number))

can be written:

print(f'Computer chose: {computer_number}.')

which improves readability.

print('----------YOU WON!----------')
print('----------YOU LOST!----------')
print('----------------------------\nYou started new game!')

String multiplication: the statements above can be replaced with:

   print(f"{10 * '-'}YOU WON!{10 * '-'}")
   print(f"{10 * '-'}YOU LOST!{10 * '-'}")
   print(f"{30 * '-'}\nYou started a new game!")
\$\endgroup\$
  • \$\begingroup\$ thank you for answer and advices. \$\endgroup\$ – HelloBT Sep 3 '19 at 12:50
  • \$\begingroup\$ Poor advice. """docstrings""" are for the generation of automatic documentation for the use of public functions. You should not write docstrings for each and every function / class / module you write; only the ones intended to be reusable by other coders. # comments should be used to document private functions and implementation details not required for the use of the function. replace_board, change_number and check_win all look like private methods, which should be commented and named with a leading underscore; they do not need docstrings. \$\endgroup\$ – AJNeufeld Sep 3 '19 at 13:51
  • \$\begingroup\$ @HelloBT As AJNeufeld pointed out i made a mistake, the isinstance check i indicated is falsy, to be replaced by while not number.isdigit() and same goes for the comments, The general conclusion: if functions are public, docstrings should be included. \$\endgroup\$ – user203258 Sep 3 '19 at 17:11

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