Große Klasse mit einer Verantwortung

Ich habe eine 2500-Linien- CharacterKlasse, die:

• Verfolgt den internen Status des Charakters im Spiel.
• Lädt diesen Zustand und behält ihn bei.
• Verarbeitet ~ 30 eingehende Befehle (normalerweise = leitet sie an weiter Game, aber einige schreibgeschützte Befehle werden sofort beantwortet).
• Erhält ca. 80 Anrufe Gamebezüglich der durchgeführten Aktionen und der relevanten Aktionen anderer.

Es scheint mir, dass Characteres eine einzige Aufgabe ist: den Zustand des Charakters zu verwalten und zwischen eingehenden Befehlen und dem Spiel zu vermitteln.

Es gibt noch einige andere Verantwortlichkeiten, die bereits aufgeschlüsselt wurden:

• Characterhat eine, in Outgoingdie es ruft, um ausgehende Updates für die Client-Anwendung zu generieren.
• Characterhat eine Timerwelche spuren wann es das nächste mal erlaubt ist etwas zu machen. Eingehende Befehle werden dagegen validiert.

Meine Frage ist also, ist es akzeptabel, eine so große Klasse unter SRP und ähnlichen Prinzipien zu haben? Gibt es bewährte Methoden, um die Arbeit zu erleichtern (z. B. das Aufteilen von Methoden in separate Dateien)? Oder fehlt mir etwas und gibt es wirklich eine gute Möglichkeit, es aufzuteilen? Mir ist klar, dass dies sehr subjektiv ist und ich möchte Feedback von anderen.

Hier ist ein Beispiel:

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)

Bei meinen Versuchen, SRP auf ein Problem anzuwenden, stelle ich im Allgemeinen fest, dass ein guter Weg, sich an eine Einzelverantwortung pro Klasse zu halten, darin besteht, Klassennamen zu wählen, die auf ihre Verantwortung hinweisen, da es oft hilfreich ist, klarer darüber nachzudenken, ob eine Funktion vorliegt gehört wirklich in diese Klasse.

Darüber hinaus glaube ich , dass einfache Substantive wie Character(oder Employee, Person, Car, Animal, etc.) oft sehr schlecht Klassennamen machen , weil sie wirklich die beschreiben Entitäten (Daten) in der Anwendung, und wenn sie als Klassen behandelt es oft zu einfach , um am Ende mit etwas sehr aufgeblähtes.

Ich finde, dass 'gute' Klassennamen in der Regel Bezeichnungen sind, die einen bestimmten Aspekt des Verhaltens Ihres Programms sinnvoll wiedergeben - dh wenn ein anderer Programmierer den Namen Ihrer Klasse sieht, gewinnt er bereits eine grundlegende Vorstellung über das Verhalten / die Funktionalität dieser Klasse.

Als Faustregel neige ich dazu, Entitäten als Datenmodelle und Klassen als Repräsentanten des Verhaltens zu betrachten. (Obwohl natürlich die meisten Programmiersprachen classfür beide ein Schlüsselwort verwenden, ist die Idee, 'einfache' Entitäten vom Anwendungsverhalten zu trennen, sprachneutral)

Angesichts der Aufschlüsselung der verschiedenen Verantwortlichkeiten, die Sie für Ihre Charakterklasse genannt haben, würde ich mich allmählich Klassen zuwenden, deren Namen auf den Anforderungen beruhen, die sie erfüllen. Beispielsweise:

• Stellen Sie sich eine CharacterModelEntität vor, die kein Verhalten aufweist und einfach den Status Ihrer Charaktere beibehält (Daten enthält).
• Berücksichtigen Sie für Persistenz / IO Namen wie CharacterReaderund CharacterWriter (oder vielleicht ein CharacterRepository/ CharacterSerialiser/ etc).
• Überlegen Sie, welche Muster zwischen Ihren Befehlen bestehen. Wenn Sie 30 Befehle haben, haben Sie möglicherweise 30 separate Verantwortlichkeiten. Einige davon mögen sich überschneiden, aber sie scheinen ein guter Kandidat für die Trennung zu sein.
• Überlegen Sie, ob Sie dasselbe Refactoring auch auf Ihre Aktionen anwenden können - wiederum können 80 Aktionen bis zu 80 separate Verantwortlichkeiten vorschlagen, möglicherweise auch mit einigen Überschneidungen.
• Die Trennung von Befehlen und Aktionen kann auch zu einer anderen Klasse führen, die für das Ausführen / Auslösen dieser Befehle / Aktionen verantwortlich ist. Vielleicht eine Art von CommandBrokeroder ActionBrokerdie sich so verhält wie die "Middleware" Ihrer Anwendung, die diese Befehle und Aktionen zwischen verschiedenen Objekten sendet / empfängt / ausführt

Denken Sie auch daran, dass nicht alles, was mit Verhalten zu tun hat, notwendigerweise Teil einer Klasse sein muss. Sie können beispielsweise eine Karte / ein Wörterbuch mit Funktionszeigern / Delegaten / Abschlüssen verwenden, um Ihre Aktionen / Befehle zu kapseln, anstatt Dutzende zustandsloser Klassen mit einer Methode zu schreiben.

Es ist ziemlich üblich, 'Command Pattern'-Lösungen zu sehen, ohne Klassen zu schreiben, die mit statischen Methoden erstellt wurden, die eine Signatur / Schnittstelle gemeinsam nutzen:

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

Schließlich gibt es keine festen Regeln dafür, wie weit Sie gehen sollten, um eine einzelne Verantwortung zu erreichen. Komplexität um der Komplexität willen ist keine gute Sache, aber Megalithklassen neigen dazu, an sich ziemlich komplex zu sein. Ein Hauptziel von SRP und der anderen SOLID-Prinzipien ist es, Struktur und Konsistenz zu gewährleisten und den Code wartungsfreundlicher zu machen - was häufig zu einer Vereinfachung führt.

Sie können immer eine abstraktere Definition einer "Verantwortung" verwenden. Das ist keine sehr gute Methode, um diese Situationen zu beurteilen, zumindest solange Sie nicht viel Erfahrung damit haben. Beachten Sie, dass Sie leicht vier Aufzählungspunkte gemacht haben, die ich einen besseren Ausgangspunkt für Ihre Klassengranularität nennen würde. Wenn Sie dem SRP wirklich folgen, ist es schwierig, solche Stichpunkte zu machen.

Eine andere Möglichkeit besteht darin, sich Ihre Klassenmitglieder anzusehen und sich anhand der Methoden, die sie tatsächlich verwenden, abzuspalten. Machen Sie beispielsweise eine Klasse aus allen tatsächlich verwendeten Methoden self.timer, eine weitere Klasse aus allen tatsächlich verwendeten Methoden self.outgoingund eine weitere Klasse aus dem Rest. Machen Sie aus Ihren Methoden eine weitere Klasse, die einen DB-Verweis als Argument verwendet. Wenn Ihre Klassen zu groß sind, gibt es oft solche Gruppierungen.

Haben Sie keine Angst, es kleiner aufzuteilen, als Sie als Experiment für sinnvoll halten. Dafür ist die Versionskontrolle da. Der richtige Gleichgewichtspunkt ist viel einfacher zu erkennen, wenn man zu weit gegangen ist.

Die Definition von "Verantwortung" ist notorisch vage, aber sie wird etwas weniger vage, wenn man sie als "Grund zur Veränderung" ansieht. Noch vage, aber etwas, das man etwas direkter analysieren kann. Gründe für Änderungen hängen von Ihrer Domain und der Art und Weise ab, in der Ihre Software verwendet wird. Spiele sind jedoch gute Beispiele, da Sie hierüber vernünftige Annahmen treffen können. In Ihrem Code zähle ich in den ersten fünf Zeilen fünf verschiedene Verantwortlichkeiten:

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

Ihre Implementierung ändert sich, wenn sich die Spielanforderungen auf eine der folgenden Arten ändern:

1. Die Vorstellung, was ein "Spiel" ausmacht, ändert sich. Dies ist möglicherweise am unwahrscheinlichsten.
2. Wie Sie Änderungen an Lebenspunkten messen und nachverfolgen
3. Ihr Angriffssystem ändert sich
4. Ihr Punktesystem ändert sich
5. Ihr Zeitsystem ändert sich

Sie laden aus Datenbanken, lösen Angriffe auf, verknüpfen sich mit Spielen, planen Dinge; Mir scheint, die Liste der Verantwortlichkeiten ist bereits sehr lang, und wir haben nur einen kleinen Teil Ihrer CharacterKlasse gesehen. Die Antwort auf einen Teil Ihrer Frage lautet also nein: Ihre Klasse folgt mit ziemlicher Sicherheit nicht der SRP.

Ich würde jedoch sagen, dass es Fälle gibt, in denen es unter SRP akzeptabel ist, eine Klasse mit 2.500 Zeilen oder länger zu haben. Einige Beispiele könnten sein:

• Eine hochkomplexe, aber genau definierte mathematische Berechnung, die genau definierte Eingaben verwendet und genau definierte Ausgaben zurückgibt. Dies könnte hochoptimierter Code sein, der Tausende von Zeilen benötigt. Bewährte mathematische Methoden für genau definierte Berechnungen haben nicht viele Gründe, sich zu ändern.
• Eine Klasse, die als Datenspeicher fungiert, z. B. eine Klasse, die nur yield return <N>die ersten 10.000 Primzahlen oder die 10.000 häufigsten englischen Wörter enthält. Es gibt mögliche Gründe, warum diese Implementierung dem Abrufen aus einem Datenspeicher oder einer Textdatei vorgezogen wird. Diese Klassen haben nur sehr wenige Gründe, sich zu ändern (z. B. brauchen Sie mehr als 10.000).

Wann immer Sie gegen eine andere Entität arbeiten, können Sie ein drittes Objekt einführen, das stattdessen die Handhabung übernimmt.

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)

Hier können Sie einen "AttackResolver" oder etwas Ähnliches einführen, das das Versenden und Sammeln von Statistiken übernimmt. Geht es bei dem Angriff hier nur um den Charakterstatus, macht er mehr?

Sie können auch den Status erneut aufrufen und sich fragen, ob ein Teil des Status, den Sie haben, wirklich für den Charakter erforderlich ist. 'successful_attack' klingt wie etwas, das Sie möglicherweise auch in einer anderen Klasse verfolgen könnten.