случайный распределитель команды

Есть идеи, как сократить и улучшить программу? он возьмет список из 22 человек, разделит их на две команды и вернет имена и команды

import random
# list of teams
teams = ['A', 'B']
# list of players which is inculded of 22 people
players = ['a', 'b', 'c', 'd', 'e',
           'f', 'g', 'h', 'i', 'j',
           'k', 'l', 'm', 'n', 'o',
           'p', 'q', 'r', 's', 't',
           'u', 'v']

# takes the list of names


class Human:
    def __init__(self, name):
        self.name = name

# distribure names and people to teams randomly and it inherits names from Human class


class Player(Human):
    def get_team(self):
        teams = dict()
        random.shuffle(self.name)
        teams['A'] = self.name[0::2]
        teams['B'] = self.name[1::2]
        return teams


my_team = Player(players)
my_team = my_team.get_team()
for team, name in my_team.items():
    print(team, name)

Концепция хороша, но есть возможности для улучшения. Вот некоторые проблемы, которые я вижу:

Вводящие в заблуждение имена

Ясные имена очень важны в кодировании, потому что они позволяют другим людям (включая вас в будущем) понять то, что вы написали, легко или сложно. Некоторые из использованных вами имен не соответствуют тому, что на самом деле делает код, что сбивает с толку.

• По названию Human класс, я бы ожидал, что экземпляр этого класса будет представлять один человек, но его используют не так (см. ниже).
• По названию Player класс, и то, как он расширяется Human, Я бы ожидал, что экземпляр этого класса будет представлять один игрок; если да, то должно быть 22 экземпляра класса или один объект для каждого имени в players список. Вместо этого он, кажется, представляет все игроки сразу, что не подразумевает его название.
• Я ожидал name атрибут Human хранить одно имя, но Player заполняет весь список имен игроков в этот атрибут.

Как минимум, эти имена должны быть во множественном числе (Humans, names, Playersи т. д.), чтобы отразить их содержание. Еще лучше было бы дать Player имя, которое описывает, что он на самом деле делает или за что отвечает в программе.

Ненужная иерархия, часть 1: только один подкласс

Вы упомянули в комментарии, что пытаетесь практиковать свои навыки ООП, поэтому «[you] хочу заставить [yourself] использовать классы, даже если это звучит глупо «. Практика — это хорошо, но частью хорошего ООП является понимание того, почему и как эффективно использовать классы и объекты … и это включает в себя знание того, когда нет использовать их, или когда не использовать определенные их части.

Например: зачем вам нужны оба Human класс и Player подкласс? В программе, которая у вас есть сейчас, единственные люди — игроки; следовательно, наличие двух классов не дает никаких преимуществ. Вы могли бы делать все в Player класс и бросить Human класс вообще, или наоборот. С другой стороны, если у вас уже есть планы расширить свою программу, включив в нее не— игроки (например, тренеры, судьи), затем Human суперкласс может иметь смысл. Структура кода должна соответствовать вашему варианту использования.

Ненужная иерархия, часть 2: суперкласс ничего не делает

Подкласс должен иметь «тип» или «вид» отношения к своему суперклассу, например Square это своего рода Rectangle, или Dog это своего рода Animal. Проблема в том, что ваш код не сильно выигрывает от определения Player как своего рода Human. С Human не имеет определенного поведения и почти не имеет данных, наследование от него не дает много преимуществ. Было бы проще и гибче использовать композицию вместо наследования, создав или импортировав Human объект внутри то Player объект.

Ненужное использование внутренних атрибутов состояния / объекта

Одна из приятных особенностей объектов заключается в том, что они могут создавать и поддерживать внутреннее состояние — небольшой фрагмент данных, который он скрывает от остальной части программы. (Разделение отдельных вещей называется инкапсуляция или же разделение проблем, и это значительно упрощает анализ программ и упрощает их отладку.) В Python внутреннее состояние хранится в атрибутах объекта, например Humanс self.name. Но вы должны знать, когда использовать внутреннее состояние, а когда нет. Хранение нескольких копий одних и тех же данных в разных местах или нескольких ссылок на один и тот же объект данных может вызвать путаницу и привести к ошибкам.

Ваш get_team() принимает список игроков, разделяет игроков между двумя известными командами, а затем возвращает dict() с этой информацией. Этот метод вызывается только один раз во всей вашей программе. Итак, в чем преимущество Player объект хранит список имен игроков внутри себя? В текущем коде нет никакого преимущества. Вы можете легко изменить get_team() в метод класса (который может быть вызван без создания экземпляра объекта), который принимает имена игроков в качестве параметра.

Жестко запрограммированные предположения

# list of teams
teams = ['A', 'B']

Поскольку вы определили это teams list в верхней части кода, я бы предположил, что редактирование этого списка изменит вывод программы. Но когда я приглядываюсь, я вижу, что этот список даже не используется. Ваш get_team() Метод явно написан для разделения игроков на две команды с именами «A» и «B», независимо от того, что находится в списке выше.

        teams['A'] = self.name[0::2]
        teams['B'] = self.name[1::2]
        return teams

Это не очень гибко, особенно если вы хотите импортировать имена команд извне программы (например, из пользовательского ввода или из файла конфигурации). Лучше всего взять список команд в качестве входных данных, подсчитать количество команд и затем соответствующим образом разделить игроков.

Вот как я могу это написать:

TEAMS = ('A', 'B')  # Replaced mutable list with immutable tuple.
                    # Names of constants should be in all-caps.
PLAYERS = ('a', 'b', 'c', 'd', 'e',
           'f', 'g', 'h', 'i', 'j',
           'k', 'l', 'm', 'n', 'o',
           'p', 'q', 'r', 's', 't',
           'u', 'v')

class TeamAssigner:
    def assign_players_to_teams(teams, players):
        players_on_teams = dict()
        randomized_players = random.sample(players, len(players))  # changed to .sample because .shuffle doesn't work on tuples / immutable sequences
        for i, team in enumerate(teams):
            players_on_teams[team] = randomized_players[i::len(teams)]
        return players_on_teams

my_teams = TeamAssigner.assign_players_to_teams(TEAMS, PLAYERS)
for team, name in my_teams.items():
    print(team, name)

Конечно, это только то, что я написал бы, если бы не планировал расширять программу в будущем. Если у вас есть планы расширить программу и заставить ее делать больше, то некоторые из этих решений могут быть неправильными. Например, если вам нужна возможность перемещать игроков из одной команды в другую. после начальное разделение, тогда вам понадобится класс, который сделал сохранять внутреннее состояние того, кто в какой команде …

Честно говоря, ваш код уже довольно компактен. На самом деле есть только 2 области, которые, я думаю, можно сократить.

1. В get_team метод Player.

    def get_team(self):
        random.shuffle(self.name)
        return {'A':self.name[0::2], 'B':self.name[1::2]}

Если вы хотите улучшить читаемость, вы можете установить словарь на team а затем вернуться team.

2. Последняя пара строк.

my_team = Player(players).get_team()
for team, name in my_team.items():
    print(team, name)

Эта проблема действительно не нуждается в классе для ее решения. Джека Дидериха Прекратить писать классы здесь очень актуален, и, как и в примерах, показанных в разговоре, Human и Player наверное, вообще не должно быть занятий.

Все, что нам нужно, это функция, которая принимает список игроков и возвращает команды:

import random

def get_teams(players):
    randomly_ordered_players = random.sample(players, len(players))
    return {
        "A": randomly_ordered_players[0::2],
        "B": randomly_ordered_players[1::2],
    }

Обратите внимание, что мы используем random.sample здесь, который возвращает новый случайным образом упорядоченный список вместо перетасовки списка на месте. Это чище, потому что мы не изменяем список ввода.

Мы можем сделать функцию более гибкой, взяв список названий команд, чтобы мы могли разделить игроков на любое количество команд. Это решение @ MJ713 с использованием понимание диктовки:

import random

def get_teams(teams, players):
    randomly_ordered_players = random.sample(players, len(players))
    number_of_teams = len(teams)
    return {
        teams[i]: randomly_ordered_players[i::number_of_teams]
        for i in range(number_of_teams)
    }

Пример:

for team_name, players in get_teams(
    ["A", "B"], ["a", "b", "c", "d", "e", "f", "g", "h"]
).items():
    print(team_name, players)

A ['e', 'f', 'a', 'g']
B ['h', 'b', 'd', 'c']

for team_name, players in get_teams(
    ["A", "B", "C", "D"], ["a", "b", "c", "d", "e", "f", "g", "h"]
).items():
    print(team_name, players)

A ['d', 'b']
B ['e', 'g']
C ['h', 'c']
D ['f', 'a']

Это может быть хорошее приложение для grouper() из Рецепты Itertools

from itertools import zip_longest
from random import shuffle

players = ['a', 'b', 'c', 'd', 'e',
       'f', 'g', 'h', 'i', 'j',
       'k', 'l', 'm', 'n', 'o',
       'p', 'q', 'r', 's', 't',
       'u', 'v']


def grouper(iterable, n, fillvalue=None):
    "Collect data into fixed-length chunks or blocks"
    # grouper('ABCDEFG', 3, 'x') --> ABC DEF Gxx"
    args = [iter(iterable)] * n
    return zip_longest(*args, fillvalue=fillvalue)

shuffle(players)

team_a, team_b = grouper(players, 11, '')