Tengo una Character
clase de línea 2500 que:
- Rastrea el estado interno del personaje en el juego.
- Carga y persiste ese estado.
- Maneja ~ 30 comandos entrantes (generalmente = los reenvía al
Game
, pero algunos comandos de solo lectura se responden de inmediato). - Recibe ~ 80 llamadas de
Game
acciones relacionadas con lo que toma y acciones relevantes de otros.
Me parece que Character
tiene una responsabilidad única: administrar el estado del personaje, mediando entre los comandos entrantes y el Juego.
Hay algunas otras responsabilidades que ya se han desglosado:
Character
tiene unaOutgoing
llamada para generar actualizaciones salientes para la aplicación cliente.Character
tiene unTimer
seguimiento que la próxima vez que se le permite hacer algo. Los comandos entrantes se validan contra esto.
Entonces mi pregunta es, ¿es aceptable tener una clase tan grande bajo SRP y principios similares? ¿Existen mejores prácticas para hacerlo menos engorroso (por ejemplo, tal vez dividir los métodos en archivos separados)? ¿O me estoy perdiendo algo y hay realmente una buena manera de dividirlo? Me doy cuenta de que esto es bastante subjetivo y me gustaría recibir comentarios de otros.
Aquí hay una muestra:
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)
object-oriented
class-design
single-responsibility
usuario35358
fuente
fuente
db.save_character_data(self, health, self.successful_attacks, self.points)
¿ Querías decirself.health
verdad?Respuestas:
En mis intentos de aplicar SRP a un problema, generalmente encuentro que una buena manera de apegarse a una responsabilidad única por clase es elegir nombres de clase que aluden a su responsabilidad, porque a menudo ayuda a pensar más claramente si alguna función realmente 'pertenece' a esa clase.
Por otra parte, siento que los sustantivos simples, tales como
Character
(oEmployee
,Person
,Car
,Animal
, etc.) a menudo hacen los nombres de clases muy pobres porque realmente describen las entidades (datos) en su aplicación, y cuando son tratados como clases es a menudo demasiado fácil acabar con Algo muy hinchado.Creo que los nombres de clase "buenos" tienden a ser etiquetas que transmiten de manera significativa algún aspecto del comportamiento de su programa, es decir, cuando otro programador ve el nombre de su clase, ya obtienen una idea básica sobre el comportamiento / funcionalidad de esa clase.
Como regla general, tiendo a pensar en las Entidades como modelos de datos, y en las Clases como representantes del comportamiento. (Aunque, por supuesto, la mayoría de los lenguajes de programación usan una
class
palabra clave para ambos, pero la idea de mantener las entidades 'simples' separadas del comportamiento de la aplicación es neutral en cuanto al lenguaje)Dado el desglose de las diversas responsabilidades que ha mencionado para su clase de personaje, comenzaría a inclinarme hacia clases cuyos nombres se basan en el requisito que cumplen. Por ejemplo:
CharacterModel
entidad que no tiene comportamiento y simplemente mantiene el estado de sus Personajes (contiene datos).CharacterReader
yCharacterWriter
(o tal vez unCharacterRepository
/CharacterSerialiser
/ etc).CommandBroker
oActionBroker
que se comporta como "middleware" de la aplicación de envío / recepción / ejecutar esos comandos y acciones entre diferentes objetosTambién recuerde que no todo lo relacionado con el comportamiento necesariamente debe existir como parte de una clase; por ejemplo, podría considerar usar un mapa / diccionario de punteros / delegados / cierres de funciones para encapsular sus acciones / comandos en lugar de escribir docenas de clases de un solo método sin estado.
Es bastante común ver soluciones de 'patrón de comando' sin escribir ninguna clase que se construya utilizando métodos estáticos que compartan una firma / interfaz:
Por último, no existen reglas estrictas y rápidas sobre qué tan lejos debe llegar para lograr la responsabilidad individual. La complejidad por el bien de la complejidad no es algo bueno, pero las clases megalíticas tienden a ser bastante complejas en sí mismas. Un objetivo clave de SRP y, de hecho, de otros principios SOLIDOS es proporcionar estructura, consistencia y hacer que el código sea más fácil de mantener, lo que a menudo resulta en algo más simple.
fuente
Walk
,Attack
yDuck
. Lo que no está bien, es tenerSave
yLoad
(persistencia). SRP establece que una clase solo debe tener una responsabilidad, pero la responsabilidad del personaje es ser un personaje, no un contenedor de datos.CharacterModel
, cuya responsabilidad es ser un contenedor de datos para desacoplar las preocupaciones de la capa de datos de la capa de lógica de negocios. De hecho, aún puede ser deseableCharacter
que exista una clase de comportamiento en algún lugar también, pero con 80 acciones y 30 comandos, me inclinaría por descomponerlo aún más. La mayoría de las veces encuentro que los sustantivos de entidad son una "pista falsa" para los nombres de clase porque es difícil extrapolar la responsabilidad de un sustantivo de entidad, y es demasiado fácil para ellos convertirse en una especie de navaja suiza.Siempre puede usar una definición más abstracta de una "responsabilidad". Esa no es una muy buena manera de juzgar estas situaciones, al menos hasta que tenga mucha experiencia en ello. Observe que fácilmente hizo cuatro viñetas, lo que yo llamaría un mejor punto de partida para su granularidad de clase. Si realmente está siguiendo el SRP, es difícil hacer viñetas como esa.
Otra forma es mirar a los miembros de su clase y separarse según los métodos que realmente los usan. Por ejemplo, haga una clase de todos los métodos que realmente usan
self.timer
, otra clase de todos los métodos que realmente usanself.outgoing
, y otra clase del resto. Haga otra clase de sus métodos que toman una referencia db como argumento. Cuando sus clases son demasiado grandes, a menudo hay grupos como este.No tenga miedo de dividirlo más pequeño de lo que cree que es razonable como experimento. Para eso es el control de versiones. El punto de equilibrio correcto es mucho más fácil de ver después de llevarlo demasiado lejos.
fuente
La definición de "responsabilidad" es notoriamente vaga, pero se vuelve un poco menos vaga si la considera una "razón para cambiar". Todavía vago, pero algo que puedes analizar un poco más directamente. Las razones para el cambio dependen de su dominio y de cómo se usará su software, pero los juegos son buenos ejemplos de casos porque puede hacer suposiciones razonables al respecto. En su código, cuento cinco responsabilidades diferentes en las primeras cinco líneas:
Su implementación cambiará si los requisitos del juego cambian de alguna de estas maneras:
Estás cargando desde bases de datos, resolviendo ataques, enlazando con juegos, cronometrando cosas; Me parece que la lista de responsabilidades ya es muy larga, y solo hemos visto una pequeña parte de su
Character
clase. Entonces, la respuesta a una parte de su pregunta es no: es casi seguro que su clase no siga el SRP.Sin embargo, diría que hay casos en los que es aceptable bajo SRP tener una clase de 2.500 líneas o más. Algunos ejemplos podrían ser:
yield return <N>
los primeros 10,000 números primos, o las 10,000 palabras en inglés más comunes. Hay posibles razones por las que se preferiría esta implementación en lugar de extraer de un almacén de datos o archivo de texto. Estas clases tienen muy pocas razones para cambiar (por ejemplo, si necesita más de 10,000).fuente
Siempre que trabaje contra alguna otra entidad, podría introducir un tercer objeto que se encarga del manejo.
Aquí puede introducir un 'AttackResolver' o algo por el estilo que maneja el envío y la recopilación de estadísticas. ¿El on_attack aquí solo sobre el estado de los caracteres está haciendo más?
También puede volver a visitar el estado y preguntarse si parte del estado que tiene realmente necesita estar en el personaje. 'exitoso_ataque' suena como algo que potencialmente podría rastrear en alguna otra clase también.
fuente