У меня есть Character
класс 2500 строк, который:
- Отслеживает внутреннее состояние персонажа в игре.
- Загружает и сохраняет это состояние.
- Обрабатывает ~ 30 входящих команд (обычно = перенаправляет их на
Game
, но некоторые команды только для чтения отвечают немедленно). - Получает ~ 80 звонков
Game
о действиях, которые он предпринимает, и о действиях других.
Мне кажется, что Character
есть единственная ответственность: управлять состоянием персонажа, посредничая между поступающими командами и игрой.
Есть несколько других обязанностей, которые уже были нарушены:
Character
имеет тот,Outgoing
который он вызывает для создания исходящих обновлений для клиентского приложения.Character
имеет,Timer
который отслеживает, когда в следующий раз разрешено что-то делать. Входящие команды проверяются против этого.
Итак, мой вопрос: допустимо ли иметь такой большой класс в рамках SRP и подобных принципов? Существуют ли передовые практики для того, чтобы сделать его менее громоздким (например, возможно разделение методов на отдельные файлы)? Или я что-то упустил и есть ли хороший способ разделить это? Я понимаю, что это довольно субъективно и хотел бы получить отзывы от других.
Вот образец:
class Character(object):
def __init__(self):
self.game = None
self.health = 1000
self.successful_attacks = 0
self.points = 0
self.timer = Timer()
self.outgoing = Outgoing(self)
def load(self, db, id):
self.health, self.successful_attacks, self.points = db.load_character_data(id)
def save(self, db, id):
db.save_character_data(self, health, self.successful_attacks, self.points)
def handle_connect_to_game(self, game):
self.game.connect(self)
self.game = game
self.outgoing.send_connect_to_game(game)
def handle_attack(self, victim, attack_type):
if time.time() < self.timer.get_next_move_time():
raise Exception()
self.game.request_attack(self, victim, attack_type)
def on_attack(victim, attack_type, points):
self.points += points
self.successful_attacks += 1
self.outgoing.send_attack(self, victim, attack_type)
self.timer.add_attack(attacker=True)
def on_miss_attack(victim, attack_type):
self.missed_attacks += 1
self.outgoing.send_missed_attack()
self.timer.add_missed_attack()
def on_attacked(attacker, attack_type, damage):
self.start_defenses()
self.take_damage(damage)
self.outgoing.send_attack(attacker, self, attack_type)
self.timer.add_attack(victim=True)
def on_see_attack(attacker, victim, attack_type):
self.outgoing.send_attack(attacker, victim, attack_type)
self.timer.add_attack()
class Outgoing(object):
def __init__(self, character):
self.character = character
self.queue = []
def send_connect_to_game(game):
self._queue.append(...)
def send_attack(self, attacker, victim, attack_type):
self._queue.append(...)
class Timer(object):
def get_next_move_time(self):
return self._next_move_time
def add_attack(attacker=False, victim=False):
if attacker:
self.submit_move()
self.add_time(ATTACK_TIME)
if victim:
self.add_time(ATTACK_VICTIM_TIME)
class Game(object):
def connect(self, character):
if not self._accept_character(character):
raise Exception()
self.character_manager.add(character)
def request_attack(character, victim, attack_type):
if victim.has_immunity(attack_type):
character.on_miss_attack(victim, attack_type)
else:
points = self._calculate_points(character, victim, attack_type)
damage = self._calculate_damage(character, victim, attack_type)
character.on_attack(victim, attack_type, points)
victim.on_attacked(character, attack_type, damage)
for other in self.character_manager.get_observers(victim):
other.on_see_attack(character, victim, attack_type)
db.save_character_data(self, health, self.successful_attacks, self.points)
вы имели в видуself.health
правильно?Ответы:
В моих попытках применить SRP к проблеме, я обычно нахожу, что хороший способ придерживаться единой ответственности за класс - это выбирать имена классов, которые ссылаются на их ответственность, потому что это часто помогает более четко подумать о том, какая-то функция действительно «принадлежит» в этом классе.
Кроме того, я чувствую, что простые существительные, такие как
Character
(илиEmployee
,Person
,Car
,Animal
и т.д.) часто делаю очень плохие имена классов , потому что они действительно описывают объекты (данные) в приложении, и когда рассматриваются как классы часто бывает слишком легко в конечном итоге что-то очень раздутое.Я считаю, что «хорошие» имена классов, как правило, представляют собой метки, которые значимо передают некоторый аспект поведения вашей программы - то есть, когда другой программист видит имя вашего класса, они уже получают базовое представление о поведении / функциональности этого класса.
Как правило, я склонен думать о сущности моделями данных, а классы - представителями поведения. (Хотя, конечно, большинство языков программирования используют
class
ключевое слово для обоих, но идея сохранения «простых» сущностей отдельно от поведения приложения не зависит от языка)Учитывая распределение различных обязанностей, которые вы упомянули в отношении класса персонажа, я бы начал склоняться к классам, имена которых основаны на требованиях, которые они выполняют. Например:
CharacterModel
сущность, которая не имеет поведения и просто поддерживает состояние ваших персонажей (содержит данные).CharacterReader
иCharacterWriter
(или, возможно,CharacterRepository
/CharacterSerialiser
/ и т. Д.).CommandBroker
илиActionBroker
который ведет себя как «промежуточное ПО» вашего приложения, отправляющее / получающее / выполняющее эти команды и действия между различными объектамиТакже помните, что не все, что связано с поведением, обязательно должно существовать как часть класса; Например, вы можете рассмотреть возможность использования карты / словаря указателей / делегатов / замыканий функций для инкапсуляции ваших действий / команд, а не написания десятков классов без единого метода без сохранения состояния.
Довольно часто можно увидеть решения «шаблон команд» без написания каких-либо классов, созданных с использованием статических методов, использующих сигнатуру / интерфейс:
Наконец, не существует жестких и быстрых правил относительно того, как далеко вы должны идти, чтобы достичь единой ответственности. Сложность ради сложности не очень хорошая вещь, но мегалитические классы, как правило, сами по себе довольно сложны. Основная цель SRP и других принципов SOLID - обеспечить структуру, согласованность и сделать код более понятным, что часто приводит к чему-то более простому.
источник
Walk
,Attack
иDuck
. Что не в порядке, это иметьSave
иLoad
(настойчивость). SRP заявляет, что класс должен иметь только одну ответственность, но обязанность персонажа - быть символом, а не контейнером данных.CharacterModel
, в обязанности которого входит создание контейнера данных для отделения проблем уровня данных от уровня бизнес-логики. Это действительно может быть желательным для поведенческогоCharacter
класс существовал где-то еще, но с 80 действиями и 30 командами я бы склонялся к его дальнейшему разрушению. Большую часть времени я нахожу, что существительные сущностей являются «красной сельдью» для имен классов, потому что трудно экстраполировать ответственность от существительного сущности, и им слишком легко превратиться в своего рода швейцарский армейский нож.Вы всегда можете использовать более абстрактное определение «ответственности». Это не очень хороший способ судить об этих ситуациях, по крайней мере, пока у вас нет большого опыта в этом. Обратите внимание, что вы легко сделали четыре пункта, которые я бы назвал лучшей отправной точкой для детализации вашего класса. Если вы действительно следуете SRP, сложно сделать такие точки.
Другой способ - взглянуть на своих учеников и разделить их на основе методов, которые действительно их используют. Например, сделайте один класс из всех методов, которые фактически используют
self.timer
, другой класс из всех методов, которые фактически используютself.outgoing
, и другой класс из остатка. Сделайте еще один класс из ваших методов, которые в качестве аргумента принимают ссылку на базу данных. Когда ваши классы слишком большие, такие группы часто бывают.Не бойтесь разделить его на части меньше, чем вы считаете разумным в качестве эксперимента. Вот для чего нужен контроль версий. Правильную точку равновесия гораздо легче увидеть, если зайти слишком далеко.
источник
Определение «ответственности» общеизвестно расплывчато, но становится немного менее расплывчатым, если вы считаете его «причиной перемен». Все еще расплывчато, но кое-что вы можете проанализировать немного более прямо. Причины изменений зависят от вашего домена и того, как будет использоваться ваше программное обеспечение, но игры являются хорошим примером, потому что вы можете сделать разумные предположения по этому поводу. В вашем коде я считаю пять разных обязанностей в первых пяти строках:
Ваша реализация изменится, если игровые требования изменятся одним из следующих способов:
Вы загружаете из баз данных, разрешаете атаки, связываетесь с играми, рассчитываете время; мне кажется, список обязанностей уже очень длинный, и мы видели лишь небольшую часть ваших
Character
класса. Таким образом, ответ на одну часть вашего вопроса - нет: ваш класс почти наверняка не следует SRP.Тем не менее, я бы сказал, что есть случаи, когда в рамках SRP допустимо иметь класс 2500 строк или более. Некоторые примеры могут быть:
yield return <N>
первые 10000 простых чисел или 10000 самых распространенных английских слов. Существуют возможные причины, по которым эта реализация предпочтительнее, чем извлечение из хранилища данных или текстового файла. У этих классов очень мало причин для изменения (например, вам нужно более 10 000).источник
Всякий раз, когда вы работаете против какой-либо другой сущности, вы можете ввести третий объект, который вместо этого выполняет обработку.
Здесь вы можете ввести AttackResolver или что-то в этом роде, которое обрабатывает отправку и сбор статистики. Здесь on_attack только о состоянии персонажа, он делает больше?
Вы также можете вернуться к состоянию и спросить себя, должно ли какое-либо состояние, которое у вас есть, быть на персонаже. «success_attack» звучит как то, что вы могли бы отслеживать в другом классе.
источник