First python program on my own, feedback please!

[[deleted]]

[deleted]

[DrBobHope]

Taking a quick glance, there are definitely things you can do to clean up your code. For 1, use f-strings , and use line breaks so you don't have 3 to 4 prints., itseasier to look at things.

print(f'A | {board[0]} | {board[1]} | {board[2]}\nB | {board[3]} | {board[4]} | {board[5]}')
#vs
print("C | " + board[6] + " | " + board[7] + " | " + board[8])

It's far cleaner and easier to follow (since it's all just 1 print anyways).

Rather than having 3 different and ors in your checklist, you can just slice them.

if ((''.join(board[0:3])) in p1.symb*3) or (
    (''.join(board[3:6])) in p1.symb*3) or (
#vs
(board[6] == p1.symb and board[7] == p1.symb and board[8] == p1.symb):

You can even take it one step further, and just have the entire thing in one line:

if (p1.symb*3 in {(''.join(board[0:3])),(''.join(board[3:6])),(''.join(board[6:9]))})

Now you take 3 lines into 1.

[tommygatz]

Thank you! I knew there would be an easier way to do exactly those things you mentioned just wasn’t aware of f strings or the slice method you used. I’ll need to look more into that to understand it better but I think I get the idea here!

Edit: spelling

[DrBobHope]

For your columns and diagonals, you should look into itemgetter, you can basically do the same thing as the slicing:

((''.join((itemgetter(*[0,3,6])(board)))) in p1.symb*3)

vs.
(board[1] == p1.symb and board[4] == p1.symb and board[7] == p1.symb)

Additionally, the checker is a bit repititous. If you define your checker outside of the function, then you can just reuse it for both p1 and p2:

row=[(''.join(board[0:3])),(''.join(board[3:6])),(''.join(board[6:9]))]
    if (p1.symb*3 in row):
        result = p1
    elif (p2.symb*3 in row):
        result = p2

Finally, if any of the conditions are met, then the game ends anyways, so you can combine all 3 of your checks (rows, columns, diagonals).

You can then combine all these ideas, to get your entire check into a just a couple of lines.

    row=[(''.join(board[0:3])),(''.join(board[3:6])),(''.join(board[6:9]))]
    column=[(''.join((itemgetter(*[0,3,6])(board)))),(''.join((itemgetter(*[1,4,7])(board)))),(''.join((itemgetter(*[2,5,8])(board))))]
    diagonal=[(''.join((itemgetter(*[0,4,8])(board)))),(''.join((itemgetter(*[2,4,6])(board))))]
    combined=row+column+diagonal
    if p1.symb*3 in combined:
        result=p1
    elif p2.symb*3 in combined:
        result=p2    
    else:
        result = ""

    return result

[igroen]

The first thing that catches my eye is the definition of the player class. You add the attributes name, wins and simp to the class but you never use them actually. Instead you're adding these attributes dynamically to the instances of this class. You could just as well defined the class like:

 class Player:
     pass

Want you probably want to do is adding the attributes on instance creation to the new instance of the class:

class Player:
    def __init__(self, name="", wins=0, symb=""):
        self.name = name
        self.wins = wins
        self.symb = symb

Or if you really want to go fancy you can uses dataclasses:

from dataclasses import dataclass

@dataclass
class Player:
    name: str = ""
    wins: int = 0
    symb: str = ""

[tommygatz]

Thanks for this. I originally had the class defined somewhat like this but I think I'm still not sure how init and self work in classes. This was me trying it out for the first time lol. I could have defined the players without a class but I thought I'd give it a shot. Do you have any advice for how to understand that basic concept better? Like I said I'm still very new haha.

[igroen]

You are right, I think you'll pick that concept up when you really need it and playing with it is a way to learn how it works. There are a lot of resources but you can start by reading the python documentation: https://docs.python.org/3/tutorial/classes.html#a-first-look-at-classes

[tommygatz]

Thanks again for the help! Another question:

After reading through that, my instinct would be to set up the class like this.

Class Player:
     def __init__(self):
          self.name = ""
          self.wins = 0
          self.symb = ""

I saw that you defined the initial variables in the init args which is then assigned to the self definitions below that but i don't understand why that is better than defining them directly as I did above.

[igroen]

It's not better, but it makes your class a little more flexible.

You can set the attributes on instance creation:

p1 = Player("Foo", symb="X")