Duża klasa z jedną odpowiedzialnością

Mam Characterklasę linii 2500, która:

• Śledzi wewnętrzny stan postaci w grze.
• Ładuje i utrzymuje ten stan.
• Obsługuje ~ 30 przychodzących poleceń (zwykle = przekazuje je do Game, ale niektóre polecenia tylko do odczytu są natychmiast reagowane).
• Odbiera ~ 80 połączeń od Gamepodejmowanych działań i odpowiednich działań innych osób.

Wydaje mi się, że Characterma jeden obowiązek: zarządzać stanem postaci, mediować między nadchodzącymi poleceniami a Grą.

Istnieje jeszcze kilka innych obowiązków, które zostały już określone:

• Characterma, do Outgoingktórego wzywa w celu generowania aktualizacji wychodzących dla aplikacji klienckiej.
• Characterma Timerścieżkę, która śledzi, kiedy będzie mogła coś zrobić. Polecenia przychodzące są sprawdzane pod tym kątem.

Więc moje pytanie brzmi: czy dopuszczalne jest posiadanie tak dużej klasy w ramach SRP i podobnych zasad? Czy istnieją jakieś najlepsze praktyki, aby uczynić to mniej uciążliwym (np. Może podzielić metody na osobne pliki)? A może coś mi brakuje i czy naprawdę istnieje dobry sposób, aby to rozdzielić? Zdaję sobie sprawę, że jest to dość subiektywne i chciałbym uzyskać informacje zwrotne od innych.

Oto próbka:

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)

W moich próbach zastosowania SRP do problemu na ogół uważam, że dobrym sposobem na trzymanie się pojedynczej odpowiedzialności za klasę jest wybieranie nazw klas, które nawiązują do ich odpowiedzialności, ponieważ często pomaga to jaśniej myśleć o tym, czy jakaś funkcja naprawdę „należy” do tej klasy.

Co więcej, uważam, że proste rzeczowniki takie jak Character(lub Employee, Person, Car, Animal, itd.) Często bardzo biednych nazwy klas, bo naprawdę opisać podmioty (dane) w aplikacji, a gdy traktowane jako klasy jest często zbyt łatwo skończyć z coś bardzo rozdętego.

Uważam, że „dobre” nazwy klas są zazwyczaj etykietami, które w znaczący sposób przekazują pewien aspekt zachowania twojego programu - tj. Kiedy inny programista widzi nazwę twojej klasy, już zyskuje podstawowe pojęcie o zachowaniu / funkcjonalności tej klasy.

Z reguły myślę o bytach jako o modelach danych, a klasy jako o przedstawicielach zachowania. (Chociaż oczywiście większość języków programowania używa classsłowa kluczowego w obu przypadkach, ale idea oddzielenia „prostych” encji od zachowania aplikacji jest neutralna językowo)

Biorąc pod uwagę podział różnych obowiązków, o których wspomniałeś dla swojej klasy postaci, zacznę pochylać się w kierunku klas, których nazwy oparte są na wymaganiu, które spełniają. Na przykład:

• Zastanów się nad CharacterModelbytem, ​​który nie zachowuje się i po prostu utrzymuje stan twoich Znaków (przechowuje dane).
• W przypadku trwałości / we / wy rozważ nazwy takie jak CharacterReaderi CharacterWriter (a może a CharacterRepository/ CharacterSerialiser/ etc).
• Pomyśl, jakie wzorce istnieją między twoimi poleceniami; jeśli masz 30 poleceń, potencjalnie masz 30 oddzielnych obowiązków; niektóre z nich mogą się pokrywać, ale wydają się dobrym kandydatem do separacji.
• Zastanów się, czy możesz zastosować to samo refaktoryzowanie do swoich Akcji - ponownie, 80 akcji może sugerować aż 80 osobnych obowiązków, być może również z pewnym nakładaniem się.
• Rozdzielenie poleceń i akcji może również prowadzić do powstania innej klasy odpowiedzialnej za uruchamianie / uruchamianie tych poleceń / akcji; może jakiś rodzaj CommandBrokerlub ActionBrokerktóry zachowuje się jak „oprogramowanie pośrednie” aplikacji wysyłające / odbierające / wykonujące polecenia i akcje między różnymi obiektami

Pamiętaj również, że nie wszystko związane z zachowaniem musi istnieć jako część klasy; na przykład, możesz rozważyć użycie mapy / słownika wskaźników funkcji / delegatów / zamknięć do enkapsulacji swoich działań / poleceń zamiast pisania dziesiątek bezstanowych klas jednej metody.

Dość często spotykane są rozwiązania „wzorca poleceń” bez pisania klas zbudowanych przy użyciu metod statycznych współdzielących podpis / interfejs:

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

Wreszcie, nie ma twardych i szybkich zasad określających, jak daleko należy posunąć się, aby osiągnąć pojedynczą odpowiedzialność. Złożoność ze względu na złożoność nie jest dobra, ale klasy megalityczne są same w sobie dość złożone. Kluczowym celem SRP i innych zasad SOLID jest zapewnienie struktury, spójności i ułatwienie utrzymania kodu - co często skutkuje czymś prostszym.

Zawsze możesz użyć bardziej abstrakcyjnej definicji „odpowiedzialności”. To nie jest zbyt dobry sposób na ocenę tych sytuacji, przynajmniej dopóki nie będziesz mieć w tym dużego doświadczenia. Zauważ, że z łatwością zdobyłeś cztery punkty, które nazwałbym lepszym punktem wyjścia dla szczegółowości twojej klasy. Jeśli naprawdę postępujesz zgodnie z SRP, trudno jest zdobyć takie punkty.

Innym sposobem jest przyjrzenie się członkom klasy i rozdzielenie ich na podstawie metod, które faktycznie ich używają. Na przykład utwórz jedną klasę ze wszystkich faktycznie używanych metod self.timer, inną klasę ze wszystkich faktycznie używanych metod self.outgoing, a drugą klasę z pozostałych. Utwórz kolejną klasę ze swoich metod, które przyjmują odwołanie db jako argument. Kiedy twoje zajęcia są zbyt duże, często są takie grupy.

Nie bój się podzielić go na mniejsze, niż uważasz za rozsądny eksperyment. Do tego służy kontrola wersji. Właściwy punkt równowagi jest znacznie łatwiejszy do zauważenia po przejściu za daleko.

Definicja „odpowiedzialności” jest notorycznie niejasna, ale staje się nieco mniej niejasna, jeśli uważasz ją za „powód do zmiany”. Wciąż niejasne, ale coś, co można przeanalizować nieco bardziej bezpośrednio. Przyczyny zmian zależą od domeny i sposobu użytkowania oprogramowania, ale gry to dobre przykłady, ponieważ można przyjąć rozsądne założenia. W twoim kodzie liczę pięć różnych obowiązków w pierwszych pięciu wierszach:

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

Twoja implementacja zmieni się, jeśli wymagania gry zmienią się w jeden z następujących sposobów:

1. Zmienia się pojęcie tego, co stanowi „grę”. To może być najmniej prawdopodobne.
2. Jak mierzysz i śledzisz zmiany punktów zdrowia
3. Twój system ataku zmienia się
4. Twój system punktów zmienia się
5. Twój system pomiaru czasu się zmienia

Ładujesz z baz danych, rozwiązujesz ataki, łączysz się z grami, mierzysz czas; wydaje mi się, że lista obowiązków jest już bardzo długa i widzieliśmy tylko niewielką część twojej Characterklasy. Odpowiedź na jedną część twojego pytania brzmi: nie. Twoja klasa prawie na pewno nie przestrzega SRP.

Powiedziałbym jednak, że istnieją przypadki, w których w ramach SRP dopuszczalne jest posiadanie klasy 2500 linii lub dłuższej. Niektóre przykłady mogą być:

• Bardzo złożone, ale dobrze zdefiniowane obliczenia matematyczne, które przyjmują dobrze określone dane wejściowe i zwracają dobrze określone dane wyjściowe. Może to być wysoce zoptymalizowany kod, który wymaga tysięcy linii. Sprawdzone metody matematyczne do dobrze zdefiniowanych obliczeń nie mają wielu powodów do zmiany.
• Klasa, która działa jak magazyn danych, na przykład klasa, która ma tylko yield return <N>pierwsze 10 000 liczb pierwszych lub 10 000 najczęściej używanych angielskich słów. Możliwe są powody, dla których ta implementacja byłaby lepsza niż pobieranie ze składnicy danych lub pliku tekstowego. Te klasy mają bardzo niewiele powodów do zmiany (np. Okazuje się, że potrzebujesz więcej niż 10 000).

Ilekroć pracujesz przeciwko innemu podmiotowi, możesz wprowadzić trzeci obiekt, który zajmuje się obsługą.

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)

Tutaj możesz wprowadzić „AttackResolver” lub coś podobnego do tych, które obsługują wysyłanie i zbieranie statystyk. Czy on_attack dotyczy tylko stanu postaci, czy robi więcej?

Możesz także ponownie przejść do stanu i zadać sobie pytanie, czy stan, w którym naprawdę jesteś, musi być w tej postaci. „udany_atak” brzmi jak coś, co można potencjalnie wyśledzić także w innej klasie.