clase grande con una sola responsabilidad

13

Tengo una clase Character de 2500 líneas que:

  • Sigue 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 son respondidos inmediatamente).
  • Recibe ~ 80 llamadas de Game con respecto a las acciones que toma y las acciones relevantes de otros.

Me parece que Character tiene una única responsabilidad: gestionar el estado del personaje, mediando entre los comandos entrantes y el Juego.

Hay algunas otras responsabilidades que ya se han desglosado:

  • Character tiene un Outgoing al que llama para generar actualizaciones salientes para la aplicación cliente.
  • Character tiene un Timer que realiza un seguimiento cuando se le permite hacer algo a continuación. Los comandos entrantes se validan contra esto.

Entonces, mi pregunta es, ¿es aceptable tener una clase tan grande bajo SRP y principios similares? ¿Existen prácticas recomendadas 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)
    
pregunta user35358 27.01.2017 - 22:20

4 respuestas

14

En mis intentos de aplicar el SRP a un problema, generalmente encuentro que una buena manera de apegarse a una única responsabilidad por clase es elegir nombres de clase que aludan a su responsabilidad, porque a menudo ayuda pensar más claramente sobre si alguna función realmente "pertenece" a esa clase.

Además, creo que los sustantivos simples como Character (o Employee , Person , Car , Animal , etc.) a menudo son nombres de clase muy pobres porque realmente describen el Entidades (datos) en su aplicación, y cuando se tratan como clases, a menudo es demasiado fácil terminar con algo muy inflado.

Encuentro que los "buenos" nombres de clase 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 tienen una idea básica sobre el comportamiento / funcionalidad de esa clase.

Como regla general, tiendo a pensar en Entidades como modelos de datos, y Clases como representantes de comportamiento. (Aunque, por supuesto, la mayoría de los lenguajes de programación usan una palabra clave class 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, me gustaría comenzar a inclinarme hacia las clases cuyos nombres se basan en el requisito que cumplen. Por ejemplo:

  • Considere una entidad CharacterModel que no tiene ningún comportamiento y simplemente mantiene el estado de sus caracteres (contiene datos).
  • Para persistencia / IO, considere nombres como CharacterReader y CharacterWriter (o tal vez un CharacterRepository / CharacterSerialiser / etc).
  • Piensa en qué tipo de patrones existen entre tus comandos; si tiene 30 comandos, entonces potencialmente tiene 30 responsabilidades separadas; algunos de los cuales pueden superponerse, pero parecen ser un buen candidato para la separación.
  • Considere si puede aplicar la misma refactorización a sus acciones también. De nuevo, 80 acciones pueden sugerir hasta 80 responsabilidades diferentes, también posiblemente con alguna superposición.
  • La separación de comandos y acciones también puede llevar a otra clase responsable de ejecutar / disparar esos comandos / acciones; quizás algún tipo de CommandBroker o ActionBroker que se comporte como el "middleware" de su aplicación enviando / recibiendo / ejecutando esos comandos y acciones entre diferentes objetos

También recuerda que no todo lo relacionado con el comportamiento debe necesariamente 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 usando métodos estáticos compartiendo una firma / interfaz:

 void AttackAction(CharacterModel) { ... }
 void ReloadAction(CharacterModel) { ... }
 void RunAction(CharacterModel) { ... }
 void DuckAction(CharacterModel) { ... }
 // etc.

Por último, no hay reglas duras y rápidas en cuanto a qué tan lejos debe ir para lograr una responsabilidad única. La complejidad en aras 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 los otros principios de SOLID 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.

    
respondido por el Ben Cottrell 28.01.2017 - 13:52
10

Siempre se puede usar una definición más abstracta de una "responsabilidad". Esa no es una buena manera de juzgar estas situaciones, al menos hasta que tenga mucha experiencia en ello. Tenga en cuenta que fácilmente hizo cuatro viñetas, lo que llamaría un mejor punto de partida para la granularidad de su clase. Si realmente estás siguiendo el SRP, es difícil hacer puntos como estos.

Otra forma es mirar a los miembros de tu clase y dividirte en función de los métodos que realmente los usan. Por ejemplo, cree una clase de todos los métodos que realmente usan self.timer , otra clase de todos los métodos que realmente usan self.outgoing y otra clase del resto. Haz otra clase de tus métodos que tome una referencia de db como argumento. Cuando tus 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.

    
respondido por el Karl Bielefeldt 28.01.2017 - 00:02
3

La definición de "responsabilidad" es notoriamente vaga, pero se vuelve un poco menos vaga si la consideras como una "razón para cambiar". Aún vaga, 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:

self.game = None
self.health = 1000
self.successful_attacks = 0
self.points = 0
self.timer = Timer()

Tu implementación cambiará si los requisitos del juego cambian de alguna de estas formas:

  1. La noción de lo que constituye un "juego" cambia. Esto puede ser lo menos probable.
  2. Cómo se miden y siguen los cambios de puntos de salud
  3. Cambia tu sistema de ataque
  4. Cambios en tu sistema de puntos
  5. Cambios en su sistema de tiempo

Estás cargando desde bases de datos, resolviendo ataques, enlazando con juegos, sincronizando cosas; Me parece que la lista de responsabilidades ya es muy larga, y solo hemos visto una pequeña parte de su clase Character . Entonces, la respuesta a una parte de su pregunta es no: su clase casi con seguridad no sigue el SRP.

Sin embargo, diría que hay casos en los que es aceptable para SRP tener una clase de 2,500 líneas o más. Algunos ejemplos podrían ser:

  • Un cálculo matemático altamente complejo pero bien definido que toma una entrada bien definida y devuelve una salida bien definida. Este podría ser un código altamente optimizado que necesita miles de líneas. Los métodos matemáticos comprobados para cálculos bien definidos no tienen muchas razones para cambiar.
  • Una clase que actúa como un almacén de datos, como una clase que solo tiene yield return <N> para 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 a la extracción desde un almacén de datos o archivo de texto. Estas clases tienen muy pocas razones para cambiar (por ejemplo, si encuentra que necesita más de 10,000).
respondido por el Carl Leth 02.02.2017 - 08:05
2

Cada vez que trabaje en contra de otra entidad, podría introducir un tercer objeto que se encarga de su manejo.

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)

Aquí puede introducir un 'AttackResolver' o algo parecido a eso que maneja el envío y la recopilación de estadísticas. ¿Es el on_attack aquí solo sobre el estado del personaje? ¿Está haciendo más?

También puede volver a visitar el estado y preguntarse si parte del estado que realmente necesita es estar en el personaje. 'Successful_attack' suena como algo que también podrías rastrear en otra clase.

    
respondido por el Joppe 28.01.2017 - 11:09

Lea otras preguntas en las etiquetas