Is there a cleaner more pythonic way to create class instances within class instances?

Question

I am programming a game with discord py. Each game have 10 players divided into 5 teams.

I don't really need:

• A reference for each player, so the self.BlackHero etc properties are unnecesary...

But I do need:

• Initiate 10 Player instances.
• Initiate 5 Team instances, containing the 10 Player instances.
• A list containing the 12 Player instances.

I was wondering if there's a more elegant way to write the code below? Thanks!

class Game:
  def __init__(self, title):
    self.title = title
    self.round = 1
    self.status = 'Registration'
    self.winners = ''
    self.deadline = ''
    self.registrations = {}
    self.chests = []
    self.monsters = []
    
    self.BlackHero = Player('BlackHero.png', 'black-team', 'hero', 'J6')
    self.BlackWitch = Player('BlackWitch.png', 'black-team', 'witch', 'J5')
    self.BlueHero = Player('BlueHero.png', 'blue-team', 'hero', 'A7')
    self.BlueWitch = Player('BlueWitch.png', 'blue-team', 'witch', 'A8')
    self.GreenHero = Player('GreenHero.png', 'green-team', 'hero', 'F9')
    self.GreenWitch = Player('GreenWitch.png', 'green-team', 'witch', 'G9')
    self.RedHero = Player('RedHero.png', 'red-team', 'hero', 'B0')
    self.RedWitch = Player('RedWitch.png', 'red-team', 'witch', 'A0')
    self.YellowHero = Player('YellowHero.png', 'yellow-team', 'hero', 'I0')
    self.YellowWitch = Player('YellowWitch.png', 'yellow-team', 'witch', 'H0')

    self.players = [
      self.BlackHero,
      self.BlackWitch,
      self.BlueHero,
      self.BlueWitch,
      self.GreenHero,
      self.GreenWitch,
      self.RedHero,
      self.RedWitch,
      self.YellowHero,
      self.YellowWitch
    ]
    
    self.teams = {
      'black-team':Team(self.BlackHero, self.BlackWitch),
      'blue-team':Team(self.BlueHero, self.BlueWitch),
      'green-team':Team(self.GreenHero, self.GreenWitch),
      'red-team':Team(self.RedHero, self.RedWitch),
      'yellow-team':Team(self.YellowHero, self.YellowWitch)
    }

class Player:
  def __init__(self, image, team, role, pos):
    self.username = None
    self.user_id = None
    self.image = image
    self.team = team #red/blue/green/yellow/black
    self.role = role #witch/hero
    self.position = pos
    self.new_position = pos
    self.old_target = None
    self.new_target = None
    self.has_sword = False
    self.is_frozen = False
    self.has_moved = False
    self.will_die = False
    self.status = 'ALIVE' #alive/dead

class Team:
  def __init__(self, hero, witch):
    self.hero = hero #username
    self.witch = witch #username
    self.items = []
    self.status = 'ALIVE'

It's functional though...

Answer 1

Assuming that:

1. Every Team has exactly 2 players, a hero and a witch, and
2. Every Game consists of a number of Teams indicated by different colors,

you can considerably simplify your code by adapting the different __init__ functions for better integration. E.g., all you need to determine a pair of hero and witch seems to be

• a shared color and
• a position for each of them.

Assuming they are always part of a team, it makes more sense to initiate the players in the Team constructor than in Game.

Consider transforming some redundant attributes into something less manual, e.g., I assume you manually change has_moved somewhere; I'd use a method for that so you can check if a player moved via player.has_moved() or, if you set @property as I did, player.has_moved like a regular attribute. Cf. is_dead in the Team. I assume you could also check has_sword in a similar manner by checking the items of the Team or something.

In general, it isn't necessary to initialize each and every possible attribute. E.g., you only need to set self.username = None if someone could try to access the username before it's set, which would throw an AttributeError. If that cannot happen, you can just set the attribute directly when the time has come to do so (player.username = 'Carl') even if you didn't initialize it with None beforehand.

If status is binary (dead or alive), you might want to change that to something like .is_dead and set it to True or False.

It's always a good idea to set __str__ and __repr__ for easier testing, too.

class Player:
    """
    Represents a Player. Is called by the Team constructor.
    
    color := color string, e.g. 'blue'
    role := 'hero' or 'witch'
    pos := '[A-J][0-9]' e.g. 'B4'
    """
    
    def __init__(self, color, role, pos):
        self.username = None
        self.user_id = None
        self.color = color
        self.position = pos
        self.new_position = pos
        
        # e.g. 'red' and 'hero' -> 'RedHero.png'
        self.image = '{}{}.png'.format(color.title(), role.title())
        self.team = '{}-team'.format(color) #red/blue/green/yellow/black
        self.role = role
        self.old_target = None
        self.new_target = None
        self.has_sword = False # maybe set this as @property method, too?
        self.is_frozen = False
        self.will_die = False

        # maybe set alive/dead as bool, too?
        self.is_dead = False
    
    @property
    def has_moved(self):
        return self.position != self.new_position
    
    # Optional: set string methods
    def __str__(self):
        return f'<{self.color} {self.role}>'.title()
    
    def __repr__(self):
        return repr(str(self))


class Team:
    """
    Represents a team of a certain color,
    consisting of a hero and a witch player.

    color := color string, e.g. 'blue'
    positions := Tuple of two positions, strings in the pattern '[A-J][0-9]'
    """
    def __init__(self, color, positions):        
        self.color = color
        self.positions = positions
        self.items = []
        
        roles = ['hero', 'witch']
        
        # initialize and store hero and witch
        for role, pos in zip(roles, positions):
            setattr(self, role, Player(color, role, pos))
            
        self.players = [self.hero, self.witch]

    @property
    def is_dead(self):
        """Returns True if all players are dead"""
        return all(p.is_dead for p in self.players)

    # Optional: set string methods
    def __str__(self):
        return f'<{self.color.title()} Team>'

    def __repr__(self):
        return repr(str(self))


class Game:
    """
    Represents a game with teams and players.
    """
    def __init__(self, title, colors):
        self.title = title
        self.colors = colors
        
        # you could also use some smarter heuristic 
        # to determine the positions
        self.positions = [['J6', 'J5'],
                          ['A7', 'A8'],
                          ['F9', 'G9'],
                          ['B0', 'A0'],
                          ['I0', 'H0']]
        self.round = 1
        self.status = 'Registration'
        self.winners = ''
        self.deadline = ''
        self.registrations = {}
        self.chests = []
        self.monsters = []
        
        # Builds teams for each color
        self.teams = [Team(color, pos) for color, pos in zip(self.colors, self.positions)]
        self.players = [p for team in self.teams for p in team.players]
    
    # Optional: set string methods
    def __str__(self):
        return f"<Game '{self.title}' – {len(self.teams)} teams>"
    
    def __repr__(self):
        return repr(str(self))

A new Game is initiated via

Game('Epic match name', ['black', 'blue', 'green', 'red', 'yellow'])

For a lower number of colors, the positions are consumed top-down. Adopt to your needs if that doesn't make sense.