Mam Character
klasę 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
Game
podejmowanych działań i odpowiednich działań innych osób.
Wydaje mi się, że Character
ma 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:
Character
ma, doOutgoing
którego wzywa w celu generowania aktualizacji wychodzących dla aplikacji klienckiej.Character
maTimer
ś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)
object-oriented
class-design
single-responsibility
użytkownik35358
źródło
źródło
db.save_character_data(self, health, self.successful_attacks, self.points)
miałeś na myśli,self.health
prawda?Odpowiedzi:
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
(lubEmployee
,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
class
sł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:
CharacterModel
bytem, który nie zachowuje się i po prostu utrzymuje stan twoich Znaków (przechowuje dane).CharacterReader
iCharacterWriter
(a może aCharacterRepository
/CharacterSerialiser
/ etc).CommandBroker
lubActionBroker
który zachowuje się jak „oprogramowanie pośrednie” aplikacji wysyłające / odbierające / wykonujące polecenia i akcje między różnymi obiektamiPamię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:
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.
źródło
Walk
,Attack
iDuck
. To, co nie jest w porządku, to miećSave
iLoad
(wytrwałość). SRP stwierdza, że klasa powinna mieć tylko jedną odpowiedzialność, ale odpowiedzialnością Postaci jest bycie postacią, a nie kontenerem danych.CharacterModel
, której zadaniem jest być kontenerem danych, aby oddzielić obawy warstwy danych od warstwy logiki biznesowej. Rzeczywiście może być pożądane, abyCharacter
klasa behawioralna gdzieś istniała, ale przy 80 akcjach i 30 poleceniach skłaniam się ku dalszemu rozkładowi. Przez większość czasu uważam, że rzeczowniki bytu są „czerwonym śledziem” dla nazw klas, ponieważ trudno jest ekstrapolować odpowiedzialność od rzeczownika bytu, a zbyt łatwo jest im zmienić się w rodzaj szwajcarskiego noża wojskowego.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 metodself.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.
źródło
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:
Twoja implementacja zmieni się, jeśli wymagania gry zmienią się w jeden z następujących sposobów:
Ł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
Character
klasy. 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ć:
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).źródło
Ilekroć pracujesz przeciwko innemu podmiotowi, możesz wprowadzić trzeci obiekt, który zajmuje się obsługą.
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.
źródło