Kelas besar dengan tanggung jawab tunggal

13

Saya memiliki Characterkelas 2500 baris yang:

  • Melacak status internal karakter dalam game.
  • Banyak dan teruskan kondisi itu.
  • Menangani ~ 30 perintah masuk (biasanya = meneruskannya ke Game, tetapi beberapa perintah read-only direspon dengan segera).
  • Menerima ~ 80 panggilan dari Gamemengenai tindakan yang diambil dan tindakan yang relevan dari orang lain.

Tampaknya bagi saya yang Charactermemiliki tanggung jawab tunggal: untuk mengelola keadaan karakter, memediasi antara perintah yang masuk dan Game.

Ada beberapa tanggung jawab lain yang telah dikerjakan:

  • Charactermemiliki Outgoingpanggilan ke mana untuk menghasilkan pembaruan keluar untuk aplikasi klien.
  • Charactermemiliki Timertrek mana yang selanjutnya diizinkan untuk melakukan sesuatu. Perintah yang masuk divalidasi terhadap ini.

Jadi pertanyaan saya adalah, apakah dapat diterima untuk memiliki kelas besar di bawah SRP dan prinsip serupa? Adakah praktik terbaik untuk membuatnya kurang rumit (mis. Mungkin metode pemisahan menjadi file terpisah)? Atau apakah saya kehilangan sesuatu dan apakah benar-benar ada cara yang baik untuk membaginya? Saya menyadari ini sangat subyektif dan ingin umpan balik dari orang lain.

Berikut ini contohnya:

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)
pengguna35358
sumber
1
Saya kira ini salah ketik: db.save_character_data(self, health, self.successful_attacks, self.points)Maksud Anda self.healthbenar?
candied_orange
5
Jika karakter Anda tetap pada level abstraksi yang benar, saya tidak melihat masalah. Jika di sisi lain itu benar-benar menangani semua rincian mengatakan memuat dan bertahan sendiri, maka Anda tidak mengikuti tanggung jawab tunggal. Delegasi adalah kunci di sini. Melihat karaktermu tahu tentang beberapa detail level rendah seperti timer dan semacamnya, aku merasa dia sudah tahu terlalu banyak.
Philip Stuyck
1
Kelas harus beroperasi pada satu tingkat abstraksi. Seharusnya tidak masuk ke detail misalnya menyimpan negara. Anda harus dapat menguraikan bongkahan kecil yang bertanggung jawab untuk bagian dalam. Pola perintah mungkin berguna di sini. Lihat juga google.pl/url?sa=t&source=web&rct=j&url=http://…
Piotr Gwiazda
Terima kasih atas komentar dan jawabannya. Saya pikir saya tidak cukup membusuk hal-hal, dan menempel terlalu banyak di kelas-kelas besar samar-samar. Menggunakan pola perintah sejauh ini sangat membantu. Saya juga telah membagi hal menjadi beberapa lapisan yang beroperasi pada berbagai tingkat abstraksi (mis. Soket, pesan game, perintah game). Saya membuat kemajuan!
user35358
1
Cara lain untuk mengatasi ini adalah memiliki "CharacterState" sebagai kelas, "CharacterInputHandler" sebagai yang lain, "CharacterPersistance" sebagai yang lain ...
T. Sar

Jawaban:

14

Dalam upaya saya untuk menerapkan SRP ke masalah, saya biasanya menemukan bahwa cara yang baik untuk tetap berpegang pada tanggung jawab tunggal per kelas adalah memilih nama kelas yang menyinggung tanggung jawab mereka, karena sering membantu untuk berpikir lebih jernih tentang apakah suatu fungsi benar-benar 'termasuk' di kelas itu.

Selain itu, saya merasa bahwa kata benda sederhana seperti Character(atau Employee, Person, Car, Animal, dll) sering membuat nama kelas yang sangat miskin karena mereka benar-benar menggambarkan entitas (data) dalam aplikasi Anda, dan ketika diperlakukan sebagai kelas sering kali terlalu mudah untuk berakhir dengan sesuatu yang sangat kembung.

Saya menemukan bahwa nama-nama kelas 'baik' cenderung label yang bermakna menyampaikan beberapa aspek perilaku program Anda - yaitu ketika programmer lain melihat nama kelas Anda, mereka sudah mendapatkan ide dasar tentang perilaku / fungsi kelas itu.

Sebagai aturan praktis, saya cenderung menganggap Entitas sebagai model data, dan Kelas sebagai perwakilan perilaku. (Meskipun tentu saja sebagian besar bahasa pemrograman menggunakan classkata kunci untuk keduanya, tetapi gagasan untuk menjaga entitas 'polos' terpisah dari perilaku aplikasi adalah bahasa-netral)

Mengingat uraian berbagai tanggung jawab yang Anda sebutkan untuk kelas karakter Anda, saya akan mulai condong ke kelas yang namanya didasarkan pada persyaratan yang mereka penuhi. Sebagai contoh:

  • Pertimbangkan CharacterModelentitas yang tidak memiliki perilaku dan hanya mempertahankan status Karakter Anda (menyimpan data).
  • Untuk kegigihan / IO, pertimbangkan nama-nama seperti CharacterReaderdan CharacterWriter (atau mungkin CharacterRepository/ CharacterSerialiser/ etc).
  • Pikirkan tentang pola apa yang ada di antara perintah Anda; jika Anda memiliki 30 perintah maka Anda berpotensi memiliki 30 tanggung jawab terpisah; beberapa di antaranya mungkin tumpang tindih, tetapi mereka tampak seperti kandidat yang baik untuk berpisah.
  • Pertimbangkan apakah Anda dapat menerapkan refactoring yang sama untuk Tindakan Anda juga - sekali lagi, 80 tindakan mungkin menyarankan sebanyak 80 tanggung jawab terpisah, juga mungkin dengan beberapa tumpang tindih.
  • Pemisahan perintah dan tindakan mungkin juga mengarah ke kelas lain yang bertanggung jawab untuk menjalankan / menjalankan perintah / tindakan tersebut; mungkin semacam CommandBrokeratau ActionBrokeryang berperilaku seperti "middleware" aplikasi Anda mengirim / menerima / mengeksekusi perintah dan tindakan di antara objek yang berbeda

Juga ingat bahwa tidak semua yang berhubungan dengan perilaku perlu ada sebagai bagian dari kelas; misalnya, Anda dapat mempertimbangkan menggunakan peta / kamus fungsi pointer / delegasi / penutupan untuk merangkum tindakan / perintah Anda daripada menulis lusinan kelas metode-tunggal tanpa kewarganegaraan.

Cukup umum untuk melihat solusi 'pola perintah' tanpa menulis kelas apa pun yang dibangun menggunakan metode statis berbagi tanda tangan / antarmuka:

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

Terakhir, tidak ada aturan yang keras dan cepat tentang seberapa jauh Anda harus mencapai satu tanggung jawab. Kompleksitas demi kompleksitas bukanlah hal yang baik, tetapi kelas megalitik cenderung cukup kompleks dalam diri mereka sendiri. Tujuan utama dari SRP dan prinsip SOLID lainnya adalah untuk menyediakan struktur, konsistensi, dan membuat kode lebih dapat dipelihara - yang seringkali menghasilkan sesuatu yang lebih sederhana.

Ben Cottrell
sumber
Saya pikir jawaban ini membahas inti dari masalah saya, terima kasih. Saya sudah menggunakannya untuk refactoring bagian aplikasi saya dan semuanya terlihat jauh lebih bersih sejauh ini.
user35358
1
Anda harus berhati-hati terhadap model Anemik , sangat dapat diterima jika model Karakter berperilaku seperti Walk, Attackdan Duck. Yang tidak oke, adalah memiliki Savedan Load(kegigihan). SRP menyatakan bahwa kelas seharusnya hanya memiliki satu tanggung jawab, tetapi tanggung jawab Karakter adalah menjadi karakter, bukan wadah data.
Chris Wohlert
1
@ChrisWohlert Itulah alasan untuk nama CharacterModel, yang tanggung jawabnya adalah menjadi wadah data untuk memisahkan masalah Lapisan Data dari lapisan Logika Bisnis. Memang mungkin masih diinginkan untuk Characterkelas perilaku untuk ada di suatu tempat juga, tetapi dengan 80 tindakan dan 30 perintah saya akan cenderung memecahnya lebih lanjut. Sebagian besar waktu saya menemukan bahwa kata benda entitas adalah "herring merah" untuk nama kelas karena sulit untuk meramalkan tanggung jawab dari kata benda entitas, dan itu terlalu mudah bagi mereka untuk berubah menjadi semacam pisau tentara Swiss.
Ben Cottrell
10

Anda selalu dapat menggunakan definisi "tanggung jawab" yang lebih abstrak. Itu bukan cara yang sangat baik untuk menilai situasi ini, setidaknya sampai Anda memiliki banyak pengalaman dalam hal itu. Perhatikan bahwa Anda dengan mudah membuat empat poin, yang saya sebut titik awal yang lebih baik untuk rincian kelas Anda. Jika Anda benar-benar mengikuti SRP, sulit untuk membuat poin-poin seperti itu.

Cara lain adalah dengan melihat anggota kelas Anda dan berpisah berdasarkan metode yang benar-benar menggunakannya. Misalnya, buat satu kelas dari semua metode yang benar-benar digunakan self.timer, kelas lain dari semua metode yang benar-benar digunakan self.outgoing, dan kelas lain dari sisanya. Buat kelas lain dari metode Anda yang menggunakan referensi db sebagai argumen. Ketika kelas Anda terlalu besar, sering kali ada pengelompokan seperti ini.

Jangan takut untuk membaginya menjadi lebih kecil dari yang Anda pikir masuk akal sebagai eksperimen. Untuk itulah kontrol versi. Titik keseimbangan yang tepat jauh lebih mudah dilihat setelah terlalu jauh.

Karl Bielefeldt
sumber
3

Definisi "tanggung jawab" terkenal samar-samar, tetapi menjadi sedikit kurang kabur jika Anda menganggapnya sebagai "alasan untuk berubah". Masih kabur, tetapi sesuatu yang bisa Anda analisis sedikit lebih langsung. Alasan perubahan tergantung pada domain Anda dan bagaimana perangkat lunak Anda akan digunakan, tetapi permainan adalah contoh kasus yang bagus karena Anda dapat membuat asumsi yang masuk akal tentang hal itu. Dalam kode Anda, saya menghitung lima tanggung jawab yang berbeda di lima baris pertama:

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

Implementasi Anda akan berubah jika persyaratan game berubah dengan cara berikut:

  1. Gagasan tentang apa yang merupakan "permainan" berubah. Ini mungkin yang paling tidak mungkin.
  2. Bagaimana Anda mengukur dan melacak perubahan poin kesehatan
  3. Sistem serangan Anda berubah
  4. Sistem poin Anda berubah
  5. Sistem pengaturan waktu Anda berubah

Anda memuat dari basis data, menyelesaikan serangan, menautkan dengan gim, menentukan waktu hal-hal; menurut saya daftar tanggung jawab sudah sangat lama, dan kami hanya melihat sebagian kecil dari Characterkelas Anda . Jadi jawaban untuk satu bagian dari pertanyaan Anda adalah tidak: kelas Anda hampir pasti tidak mengikuti SRP.

Namun, saya akan mengatakan ada kasus di mana itu dapat diterima di bawah SRP untuk memiliki kelas 2.500-line, atau lebih lama. Beberapa contoh dapat berupa:

  • Perhitungan matematis yang sangat kompleks tetapi terdefinisi dengan baik yang mengambil input yang terdefinisi dengan baik dan mengembalikan output yang terdefinisi dengan baik. Ini bisa menjadi kode yang sangat dioptimalkan yang membutuhkan ribuan baris. Metode matematika yang terbukti untuk perhitungan yang jelas tidak memiliki banyak alasan untuk berubah.
  • Kelas yang bertindak sebagai penyimpan data, seperti kelas yang hanya memiliki yield return <N>10.000 bilangan prima pertama, atau 10.000 kata bahasa Inggris yang paling umum. Ada beberapa kemungkinan alasan mengapa implementasi ini lebih disukai daripada menarik dari penyimpanan data atau file teks. Kelas-kelas ini memiliki sedikit alasan untuk berubah (mis. Anda merasa perlu lebih dari 10.000).
Carl Leth
sumber
2

Setiap kali Anda melawan beberapa entitas lain, Anda bisa memperkenalkan objek ketiga yang melakukan penanganan.

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)

Di sini Anda bisa memperkenalkan 'AttackResolver' atau sesuatu di sepanjang garis yang menangani pengiriman dan pengumpulan statistik. Apakah on_attack di sini hanya tentang status karakter apakah ini melakukan lebih banyak?

Anda juga dapat mengunjungi kembali negara bagian dan bertanya pada diri sendiri apakah beberapa negara bagian yang Anda miliki benar-benar perlu menjadi karakter. 'success_attack' terdengar seperti sesuatu yang berpotensi Anda lacak pada beberapa kelas lain juga.

Joppe
sumber