Apakah ini merupakan pelanggaran terhadap Prinsip Pergantian Liskov?

133

Katakanlah kita memiliki daftar entitas Tugas, dan ProjectTasksub tipe. Tugas dapat ditutup kapan saja, kecuali ProjectTasksyang tidak dapat ditutup setelah status Mulai. UI harus memastikan opsi untuk menutup permulaan ProjectTasktidak pernah tersedia, tetapi beberapa perlindungan ada dalam domain:

public class Task
{
     public Status Status { get; set; }

     public virtual void Close()
     {
         Status = Status.Closed;
     }
}

public class ProjectTask : Task
{
     public override void Close()
     {
          if (Status == Status.Started) 
              throw new Exception("Cannot close a started Project Task");

          base.Close();
     }
}

Sekarang ketika memanggil Close()suatu Tugas, ada kemungkinan panggilan akan gagal jika itu adalah ProjectTaskdengan status yang dimulai, ketika itu tidak akan jika itu adalah Tugas dasar. Tetapi ini adalah persyaratan bisnis. Itu harus gagal. Bisakah ini dianggap sebagai pelanggaran prinsip substitusi Liskov ?

Paul T Davies
sumber
14
Sempurna untuk contoh T yang melanggar substitusi liskov. Jangan gunakan warisan di sini, dan Anda akan baik-baik saja.
Jimmy Hoffa
8
Anda mungkin ingin mengubahnya ke public Status Status { get; private set; }:; jika tidak, Close()metode ini dapat diselesaikan.
Pekerjaan
5
Mungkin ini hanya contoh ini, tetapi saya melihat tidak ada manfaat materi untuk mematuhi LSP. Bagi saya, solusi dalam pertanyaan ini lebih jelas, lebih mudah dipahami, dan lebih mudah dirawat daripada yang sesuai dengan LSP.
Ben Lee
2
@ BenLee Tidak mudah memelihara. Hanya terlihat seperti itu karena Anda melihat ini secara terpisah. Ketika sistemnya besar, pastikan subtipe Tasktidak memperkenalkan ketidakcocokan yang aneh dalam kode polimorfik yang hanya diketahui Taskadalah masalah besar. LSP bukan kemauan, tetapi diperkenalkan justru untuk membantu pemeliharaan dalam sistem besar.
Andres F.
8
@ BenLee Bayangkan Anda memiliki TaskCloserproses yang closesAllTasks(tasks). Proses ini jelas tidak berusaha menangkap pengecualian; setelah semua, itu bukan bagian dari kontrak eksplisit Task.Close(). Sekarang Anda memperkenalkan ProjectTaskdan tiba-tiba Anda TaskClosermulai melempar (mungkin tidak ditangani) pengecualian. Ini masalah besar!
Andres F.

Jawaban:

174

Ya, ini merupakan pelanggaran terhadap LSP. Prinsip Pergantian Liskov mensyaratkan itu

  • Prasyarat tidak dapat diperkuat dalam subtipe.
  • Postconditions tidak dapat dilemahkan dalam subtipe.
  • Invarian tipe supertipe harus dipertahankan dalam subtipe.
  • Batasan sejarah ("aturan sejarah"). Objek dianggap dapat dimodifikasi hanya melalui metode mereka (enkapsulasi). Karena subtipe dapat memperkenalkan metode yang tidak ada dalam supertipe, pengenalan metode ini memungkinkan perubahan status pada subtipe yang tidak diizinkan dalam supertipe. Batasan sejarah melarang ini.

Contoh Anda melanggar persyaratan pertama dengan memperkuat prasyarat untuk memanggil Close()metode.

Anda dapat memperbaikinya dengan membawa pra-kondisi yang diperkuat ke tingkat atas hierarki warisan:

public class Task {
    public Status Status { get; set; }
    public virtual bool CanClose() {
        return true;
    }
    public virtual void Close() {
        Status = Status.Closed;
    }
}

Dengan menetapkan bahwa panggilan Close()valid hanya di negara bagian ketika Anda CanClose()kembali, trueAnda membuat pra-kondisi berlaku untuk Taskdan juga untuk ProjectTask, memperbaiki pelanggaran LSP:

public class ProjectTask : Task {
    public override bool CanClose() {
        return Status != Status.Started;
    }
    public override void Close() {
        if (Status == Status.Started) 
            throw new Exception("Cannot close a started Project Task");
        base.Close();
    }
}
dasblinkenlight
sumber
17
Saya tidak suka duplikasi cek itu. Saya lebih suka pengecualian melempar masuk ke Task.Close dan hapus virtual dari Close.
Euforia
4
@ Euphoric Itu benar, memiliki level atas Closemelakukan pengecekan, dan menambahkan yang dilindungi DoCloseakan menjadi alternatif yang valid. Namun, saya ingin tetap sedekat mungkin dengan contoh OP; memperbaiki itu adalah pertanyaan terpisah.
dasblinkenlight
5
@ Euphoric: Tapi sekarang tidak ada cara menjawab pertanyaan, "Bisakah tugas ini ditutup?" tanpa berusaha menutupnya. Ini secara tidak perlu memaksa penggunaan pengecualian untuk kontrol aliran. Saya akan mengakui, bagaimanapun, bahwa hal semacam ini dapat diambil terlalu jauh. Terlalu jauh, solusi semacam ini dapat menghasilkan kekacauan yang giat. Terlepas dari itu, pertanyaan OP menurut saya lebih tentang prinsip, jadi jawaban menara gading sangat tepat. +1
Brian
30
@ Brian CanClose masih ada di sana. Itu masih bisa dipanggil untuk memeriksa apakah tugas dapat ditutup. Check in Close juga harus memanggil ini.
Euforia
5
@Ehhoric: Ah, saya salah paham. Anda benar, itu membuat solusi yang jauh lebih bersih.
Brian
82

Iya. Ini melanggar LSP.

Saran saya adalah menambahkan CanClosemetode / properti ke tugas dasar, sehingga tugas apa pun dapat mengetahui apakah tugas dalam kondisi ini dapat ditutup. Itu juga bisa memberikan alasan mengapa. Dan menghapus virtual dari Close.

Berdasarkan komentar saya:

public class Task {
    public Status Status { get; private set; }

    public virtual bool CanClose(out String reason) {
        reason = null;
        return true;
    }
    public void Close() {
        String reason;
        if (!CanClose(out reason))
            throw new Exception(reason);

        Status = Status.Closed;
    }
}

public ProjectTask : Task {
    public override bool CanClose(out String reason) {
        if (Status != Status.Started)
        {
            reason = "Cannot close a started Project Task";
            return false;
        }
        return base.CanClose(out reason);
    }
}
Euforia
sumber
3
Terima kasih untuk ini, Anda mengambil contoh dasblinkenlight satu langkah lebih jauh, tapi saya suka penjelasannya dan justifikasi. Maaf saya tidak dapat menerima 2 jawaban!
Paul T Davies
Saya tertarik untuk mengetahui mengapa tanda tangannya adalah bool virtual publik CanClose (alasan di luar String) - dengan menggunakan apakah Anda hanya melakukan pemeriksaan di masa mendatang? Atau ada sesuatu yang lebih halus yang saya lewatkan?
Reacher Gilt
3
@ ReacherGilt Saya pikir Anda harus memeriksa apa yang dilakukan / ref lakukan dan membaca kode saya lagi. Anda bingung. Cukup "Jika tugas tidak dapat ditutup, saya ingin tahu mengapa."
Euforia
2
out tidak tersedia dalam semua bahasa, mengembalikan tuple (atau objek sederhana yang merangkum alasannya dan boolean akan membuatnya lebih portabel di seluruh bahasa OO meskipun dengan mengorbankan kehilangan kemudahan memiliki bool secara langsung. Karena itu, untuk bahasa yang DO mendukung, tidak ada yang salah dengan jawaban ini
Newtopian
1
Dan apakah boleh untuk memperkuat prasyarat untuk properti CanClose? Yaitu menambahkan kondisi?
John V
24

Prinsip substitusi Liskov menyatakan bahwa kelas dasar harus dapat diganti dengan sub-kelasnya tanpa mengubah sifat yang diinginkan dari program. Karena hanya ProjectTaskmenimbulkan pengecualian ketika ditutup, program harus diubah menjadi acommodate untuk itu, harus ProjectTaskdigunakan sebagai pengganti Task. Jadi itu pelanggaran.

Tetapi jika Anda memodifikasi Taskpernyataan dalam tanda tangannya bahwa hal itu dapat menimbulkan pengecualian saat ditutup, maka Anda tidak akan melanggar prinsip.

Tulains Córdova
sumber
Saya menggunakan c # yang saya pikir tidak memiliki kemungkinan ini, tapi saya tahu Java melakukannya.
Paul T Davies
2
@PaulTDavies Anda dapat menghiasi metode dengan pengecualian apa yang dilemparnya, msdn.microsoft.com/en-us/library/5ast78ax.aspx . Anda memperhatikan ini saat mengarahkan kursor ke metode dari pustaka kelas dasar Anda akan mendapatkan daftar pengecualian. Itu tidak ditegakkan, tetapi itu membuat penelepon tetap sadar.
Despertar
18

Pelanggaran LSP membutuhkan tiga pihak. Tipe T, Subtipe S, dan program P yang menggunakan T tetapi diberi instance S.

Pertanyaan Anda telah memberikan T (Tugas) dan S (ProjectTask), tetapi tidak P. Jadi pertanyaan Anda tidak lengkap dan jawabannya memenuhi syarat: Jika ada P yang tidak mengharapkan pengecualian, maka untuk P itu, Anda memiliki LSP pelanggaran. Jika setiap P mengharapkan pengecualian maka tidak ada pelanggaran LSP.

Namun, Anda memang memiliki pelanggaran SRP . Fakta bahwa keadaan tugas dapat diubah, dan kebijakan yang tugas-tugas tertentu di negara-negara tertentu harus tidak diubah ke negara-negara lain, dua tanggung jawab yang sangat berbeda.

  • Tanggung jawab 1: Mewakili tugas.
  • Tanggung jawab 2: Menerapkan kebijakan yang mengubah keadaan tugas.

Kedua tanggung jawab ini berubah karena alasan yang berbeda dan karenanya harus berada di kelas yang terpisah. Tugas harus menangani fakta sebagai tugas, dan data yang terkait dengan tugas. TaskStatePolicy harus menangani cara transisi tugas dari negara ke negara dalam aplikasi yang diberikan.

Robert Martin
sumber
2
Tanggung jawab sangat bergantung pada domain dan (dalam contoh ini) seberapa rumit status tugas dan penggantinya. Dalam hal ini, tidak ada indikasi hal itu, jadi tidak ada masalah dengan SRP. Adapun pelanggaran LSP, saya percaya kita semua berasumsi bahwa penelepon tidak mengharapkan pengecualian dan aplikasi harus menunjukkan pesan yang masuk akal alih-alih masuk ke keadaan yang salah.
Euforia
Unca 'Bob merespons? "Kami tidak layak! Kami tidak layak!". Lagi pula ... Jika setiap P mengharapkan pengecualian maka tidak ada pelanggaran LSP. TETAPI jika kita menetapkan instance T tidak bisa melempar OpenTaskException(hint, hint) dan setiap P mengharapkan pengecualian lalu apa yang dikatakan tentang kode ke antarmuka, bukan implementasi? Apa yang saya bicarakan? Saya tidak tahu Aku hanya terkejut bahwa aku mengomentari jawaban Bob Unca.
radarbob
3
Anda benar bahwa membuktikan pelanggaran LSP memerlukan tiga objek. Namun, pelanggaran LSP ada jika ada program P APAPUN yang benar tanpa adanya S tetapi gagal dengan penambahan S.
kevin cline
16

Ini mungkin atau mungkin bukan merupakan pelanggaran terhadap LSP.

Serius. Dengarkan aku.

Jika Anda mengikuti LSP, objek bertipe ProjectTaskharus berperilaku sebagai objek bertipe Taskdiharapkan berperilaku.

Masalah dengan kode Anda adalah bahwa Anda belum mendokumentasikan bagaimana objek bertipe Taskdiharapkan berperilaku. Anda memiliki kode tertulis, tetapi tidak memiliki kontrak. Saya akan menambahkan kontrak untuk Task.Close. Bergantung pada kontrak yang saya tambahkan, kode untuk ProjectTask.Closemengikuti atau tidak mengikuti LSP.

Diberikan kontrak berikut untuk Task.Close, kode untuk ProjectTask.Close tidak mengikuti LSP:

     // Behaviour: Moves the task to the closed state
     // and does not throw any Exception.
     // Default behaviour: Moves the task to the closed state
     // and does not throw any Exception.
     public virtual void Close()
     {
         Status = Status.Closed;
     }

Diberikan kontrak berikut untuk Task.Close, kode untuk ProjectTask.Close tidak mengikuti LSP:

     // Behaviour: Moves the task to the closed status if possible.
     // If this is not possible, this method throws an Exception
     // and leaves the status unchanged.
     // Default behaviour: Moves the task to the closed state
     // and does not throw any Exception.
     public virtual void Close()
     {
         Status = Status.Closed;
     }

Metode yang mungkin diganti harus didokumentasikan dalam dua cara:

  • "Perilaku" mendokumentasikan apa yang bisa diandalkan oleh klien yang tahu objek penerima adalah Task, tetapi tidak tahu kelas apa itu contoh langsungnya. Ini juga memberi tahu desainer subclass mana yang menimpa wajar dan mana yang tidak masuk akal.

  • "Perilaku default" mendokumentasikan apa yang bisa diandalkan oleh klien yang tahu bahwa objek penerima adalah contoh langsung dari Task(yaitu apa yang Anda dapatkan jika Anda gunakan new Task(). Ini juga memberi tahu perancang subkelas perilaku apa yang akan diwarisi jika mereka tidak mewarisi timpa metode ini.

Sekarang hubungan-hubungan berikut harus berlaku:

  • Jika S adalah subtipe dari T, perilaku yang didokumentasikan dari S harus memperbaiki perilaku yang didokumentasikan dari T.
  • Jika S adalah subtipe dari (atau sama dengan) T, perilaku kode S harus memperbaiki perilaku terdokumentasi dari T.
  • Jika S adalah subtipe dari (atau sama dengan) T, perilaku default S harus memperbaiki perilaku T.
  • Perilaku aktual kode untuk kelas harus memperbaiki perilaku default yang didokumentasikan.
Theodore Norvell
sumber
@ user61852 mengangkat titik bahwa Anda dapat menyatakan dalam tanda tangan metode bahwa hal itu dapat menimbulkan pengecualian, dan hanya dengan melakukan ini (sesuatu yang tidak memiliki kode efek yang sebenarnya bijaksana) Anda tidak lagi melanggar LSP.
Paul T Davies
@ PaulTDavies Anda benar. Tetapi dalam kebanyakan bahasa tanda tangan bukan cara yang baik untuk menyatakan bahwa suatu rutinitas dapat menimbulkan pengecualian. Sebagai contoh dalam OP (dalam C #, saya pikir) implementasi kedua Closetidak melempar. Jadi tanda tangan menyatakan bahwa pengecualian dapat dilemparkan - tidak mengatakan bahwa tidak akan terjadi. Java melakukan pekerjaan yang lebih baik dalam hal ini. Meski begitu, jika Anda menyatakan bahwa suatu metode mungkin menyatakan pengecualian, Anda harus mendokumentasikan keadaan di mana metode itu mungkin (atau akan). Jadi saya berpendapat bahwa untuk memastikan apakah LSP dilanggar, kita perlu dokumentasi di luar tanda tangan.
Theodore Norvell
4
Banyak jawaban di sini tampaknya mengabaikan fakta bahwa Anda tidak dapat mengetahui apakah suatu kontrak divalidasi jika Anda tidak mengetahui kontrak itu. Terima kasih atas jawaban itu.
gnasher729
Jawaban yang bagus, tetapi jawaban yang lain juga bagus. Mereka menyimpulkan bahwa kelas dasar tidak membuang pengecualian karena tidak ada di kelas itu yang menunjukkan tanda-tanda itu. Jadi program, yang menggunakan kelas dasar seharusnya tidak mempersiapkan pengecualian.
inf3rno
Anda benar bahwa daftar pengecualian harus didokumentasikan di suatu tempat. Saya pikir tempat terbaik ada dalam kode. Ada pertanyaan terkait di sini: stackoverflow.com/questions/16700130/... Tapi Anda dapat melakukan ini tanpa anotasi, dll ... juga, cukup tuliskan sesuatu seperti if (false) throw new Exception("cannot start")ke kelas dasar. Kompiler akan menghapusnya, dan masih berisi kode apa yang dibutuhkan. Btw. kami masih memiliki pelanggaran LSP dengan solusi ini, karena prasyarat masih diperkuat ...
inf3rno
6

Ini bukan pelanggaran Prinsip Pergantian Liskov.

Prinsip Pergantian Liskov mengatakan:

Biarkan q (x) menjadi properti dapat dibuktikan tentang obyek x tipe T . Biarkan S menjadi subtipe dari T . Tipe S melanggar Prinsip Pergantian Liskov jika objek y dari tipe S ada, sehingga q (y) tidak dapat dibuktikan.

Alasannya, mengapa penerapan subtipe Anda bukan merupakan pelanggaran terhadap Prinsip Pergantian Liskov, cukup sederhana: tidak ada yang dapat dibuktikan tentang apa yang Task::Close()sebenarnya dilakukannya. Tentu, ProjectTask::Close()melempar pengecualian ketika Status == Status.Started, tapi begitu mungkin Status = Status.Closeddi Task::Close().

Oswald
sumber
4

Ya, itu pelanggaran.

Saya sarankan Anda memiliki hierarki Anda mundur. Jika tidak semua dekat Task, maka close()tidak termasuk dalam Task. Mungkin Anda menginginkan sebuah antarmuka, CloseableTaskyang tidak ProjectTasksdapat diimplementasikan oleh semua.

Tom G
sumber
3
Setiap Tugas dapat ditutup, tetapi tidak dalam setiap keadaan.
Paul T Davies
Pendekatan ini tampaknya berisiko bagi saya karena orang dapat menulis kode mengharapkan semua Task untuk mengimplementasikan ClosableTask, meskipun itu memodelkan masalah secara akurat. Saya terpecah antara pendekatan ini dan mesin negara karena saya benci mesin negara.
Jimmy Hoffa
Jika Tasktidak diterapkan sendiri CloseableTaskmaka mereka sedang melakukan casting yang tidak aman di suatu tempat untuk menelepon Close().
Tom G
@ TomG itulah yang saya takutkan
Jimmy Hoffa
1
Sudah ada mesin negara. Objek tidak dapat ditutup karena berada dalam kondisi yang salah.
Kaz
3

Selain menjadi masalah LSP, sepertinya menggunakan pengecualian untuk mengontrol aliran program (saya harus berasumsi bahwa Anda menangkap pengecualian sepele ini di suatu tempat dan melakukan beberapa aliran kustom daripada membiarkannya merusak aplikasi Anda).

Sepertinya ini menjadi tempat yang baik untuk menerapkan pola Negara untuk TaskState dan membiarkan objek negara mengelola transisi yang valid.

Ed Hastings
sumber
1

Saya kehilangan di sini hal penting yang terkait dengan LSP dan Desain oleh Kontrak - dalam prasyarat, adalah penelepon yang bertanggung jawab untuk memastikan prasyarat terpenuhi. Kode yang dipanggil, dalam teori DbC, seharusnya tidak memverifikasi prasyarat. Kontrak harus menentukan kapan tugas dapat ditutup (mis. CanClose mengembalikan Benar) dan kemudian kode panggilan harus memastikan prasyarat terpenuhi, sebelum memanggil Tutup ().

Ezoela Vacca
sumber
Kontrak harus menentukan perilaku apa pun yang dibutuhkan bisnis. Dalam hal ini, Tutup itu () akan memunculkan eksepsi ketika dipanggil memulai ProjectTask. Ini adalah kondisi pasca (dikatakan apa yang terjadi setelah metode dipanggil) dan memenuhinya adalah tanggung jawab kode yang disebut.
Goyo
@ Goyo Ya, tetapi seperti yang orang lain katakan pengecualian muncul dalam subtipe yang memperkuat prasyarat dan dengan demikian melanggar kontrak (tersirat) yang memanggil Tutup () hanya menutup tugas.
Ezoela Vacca
Prasyarat yang mana? Saya tidak melihat.
Goyo
@Goyo Periksa jawaban yang diterima, misalnya :) Di kelas dasar, Close tidak memiliki prasyarat, itu disebut dan menutup tugas. Namun, pada anak, ada prasyarat tentang status yang tidak dimulai. Seperti yang ditunjukkan oleh orang lain, ini adalah kriteria yang lebih kuat dan karenanya perilaku tidak dapat digantikan.
Ezoela Vacca
Sudahlah, saya menemukan prasyarat dalam pertanyaan itu. Tapi kemudian tidak ada yang salah (DbC-bijaksana) dengan kode yang disebut memeriksa pra-kondisi dan meningkatkan pengecualian ketika mereka tidak terpenuhi. Ini disebut "pemrograman defensif". Lebih lanjut, jika ada kondisi paska yang menyatakan apa yang terjadi ketika prasyarat tidak terpenuhi seperti dalam kasus ini, pelaksanaan harus memverifikasi prasyarat untuk memastikan bahwa kondisi pas terpenuhi.
Goyo
0

Ya, ini jelas merupakan pelanggaran terhadap LSP.

Beberapa orang berpendapat di sini bahwa membuat eksplisit di kelas dasar bahwa subclass dapat membuang pengecualian akan membuat ini dapat diterima, tetapi saya tidak berpikir itu benar. Tidak peduli apa yang Anda dokumentasikan di kelas dasar atau pada level abstraksi apa Anda memindahkan kode, prasyarat masih akan diperkuat dalam subkelas, karena Anda menambahkan bagian "Tidak dapat menutup tugas proyek yang dimulai" ke dalamnya. Ini bukan sesuatu yang bisa Anda selesaikan dengan solusi, Anda memerlukan model yang berbeda, yang tidak melanggar LSP (atau kita perlu melonggarkan kendala "prasyarat tidak dapat diperkuat").

Anda dapat mencoba pola dekorator jika Anda ingin menghindari pelanggaran LSP dalam kasus ini. Mungkin berhasil, saya tidak tahu.

inf3rno
sumber