Nama untuk antipattern ini? Bidang sebagai variabel lokal [ditutup]

68

Dalam beberapa kode yang saya ulas, saya melihat hal-hal yang setara dengan moral berikut ini:

public class Foo
{
    private Bar bar;

    public MethodA()
    {
        bar = new Bar();
        bar.A();
        bar = null;
    }

    public MethodB()
    {
        bar = new Bar();
        bar.B();
        bar = null;
    }
}

Bidang di barsini secara logis adalah variabel lokal, karena nilainya tidak pernah dimaksudkan untuk bertahan di seluruh pemanggilan metode. Namun, karena banyak metode yang Foomembutuhkan objek tipe Bar, pembuat kode asli baru saja membuat bidang tipe Bar.

  1. Ini jelas buruk, bukan?
  2. Apakah ada nama untuk antipattern ini?
JSB ձոգչ
sumber
60
Ya, itu yang baru. Semoga kelas ini tidak digunakan dalam konteks multithreaded, atau Anda akan memiliki waktu yang menarik di depan!
TMN
8
Ini benar-benar tidak biasa? Saya telah melihat ini berkali-kali dalam karir saya.
JSB ձոգչ
32
@ TMN Ini bukan thread aman, tapi yang lebih buruk, itu bahkan tidak reentrant. Jika ada kode di MethodAatau MethodBmenyebabkan MethodAatau MethodBdipanggil dengan cara apa pun (dan bardigunakan lagi, dalam kasus bar.{A,B}()menjadi pelaku) Anda memiliki masalah yang sama bahkan tanpa konkurensi.
73
Apakah kita harus memiliki nama untuk setiap hal bodoh yang bisa dilakukan seseorang? Sebut saja itu Ide Buruk.
Caleb
74
Sulit untuk tidak menyebutnya anti-pola privat yang menjuntai.
psr

Jawaban:

86

Ini jelas buruk, bukan?

Ya.

  • Itu membuat metode non-reentrant, yang merupakan masalah jika mereka dipanggil pada contoh yang sama secara rekursif atau dalam konteks multi-threaded.

  • Ini berarti bahwa keadaan dari satu panggilan bocor ke panggilan lain (jika Anda lupa untuk menginisialisasi ulang).

  • Itu membuat kode sulit dimengerti karena Anda harus memeriksa di atas untuk memastikan apa yang sebenarnya dilakukan kode. (Berbeda dengan menggunakan variabel lokal.)

  • Itu membuat setiap Foocontoh lebih besar dari yang seharusnya. (Dan bayangkan melakukan ini untuk variabel N ...)

Apakah ada nama untuk antipattern ini?

IMO, ini tidak layak disebut antipattern. Ini hanya kebiasaan pengkodean yang buruk / penyalahgunaan kode konstruksi / kode Java. Ini adalah hal yang mungkin Anda lihat dalam kode sarjana ketika siswa telah bolos kuliah dan / atau tidak memiliki bakat untuk pemrograman. Jika Anda melihatnya dalam kode produksi, itu adalah tanda bahwa Anda perlu melakukan lebih banyak ulasan kode ...


Sebagai catatan, cara yang benar untuk menulis ini adalah:

public class Foo
{
    public methodA()
    {
        Bar bar = new Bar();  // Use a >>local<< variable!!
        bar.a();
    }

    // Or more concisely (in this case) ...
    public methodB()
    {
        new Bar().b();
    }
}

Perhatikan bahwa saya juga telah memperbaiki metode dan nama variabel agar sesuai dengan aturan gaya yang diterima untuk pengidentifikasi Java.

Stephen C
sumber
11
Saya akan mengatakan "Jika Anda melihatnya dalam kode produksi, itu adalah tanda bahwa Anda harus memeriksa kembali proses perekrutan Anda "
Beta
@ Beta - itu juga, meskipun Anda harus dapat memperbaiki kebiasaan pengkodean yang buruk seperti itu dengan peninjauan kode yang kuat.
Stephen C
+1 untuk sedikit tentang ulasan kode lainnya. Terlepas dari apa yang dipikirkan orang, meningkatkan praktik perekrutan tidak akan mencegah masalah seperti ini - perekrutan adalah omong kosong, yang terbaik yang dapat Anda lakukan adalah memuat dadu sesuai keinginan Anda.
mattnz
@StephenC "Itu membuat metode non-reentrant, yang merupakan masalah jika mereka dipanggil pada contoh yang sama secara rekursif atau dalam konteks multi-threaded." . Saya tahu apa itu reentrancy tetapi tidak bisa melihat titik untuk ini di sini karena metode tidak menggunakan kunci? Bisakah Anda jelaskan.
Geek
@ Geek - Anda terlalu fokus pada contoh spesifik. Saya berbicara tentang kasus umum di mana metode ini dapat dipanggil dari banyak utas, atau secara rekursif.
Stephen C
39

Saya akan menyebutnya ruang lingkup besar yang tidak perlu untuk variabel. Pengaturan ini juga memungkinkan untuk kondisi balapan jika beberapa utas mengakses MethodA dan MethodB.

Th 00 mÄ s
sumber
12
Secara khusus, saya pikir "ruang lingkup variabel" adalah nama dari masalah di sini. Orang ini perlu mempelajari variabel lokal apa dan bagaimana / kapan menggunakannya. Pola positif atau aturan umum adalah menggunakan ruang lingkup terkecil yang tersedia untuk Anda untuk setiap variabel dan hanya memperluasnya sesuai kebutuhan. Agar jelas, kesalahan ini bukan hanya gaya buruk. Pengkodean kondisi lomba seperti ini adalah kesalahan.
GlenPeterson
30

Ini adalah kasus khusus dari pola umum yang dikenal sebagai global doorknobbing. Saat membangun rumah, tergoda untuk membeli hanya satu kenop pintu dan membiarkannya tergeletak di sekitar. Ketika seseorang ingin menggunakan pintu atau lemari, mereka hanya mengambil gagang pintu global itu. Jika gagang pintu mahal, itu bisa menjadi desain yang bagus.

Sayangnya, ketika teman Anda datang dan kenop pintu secara bersamaan digunakan di dua tempat yang berbeda, kenyataan hancur. Jika gagang pintu begitu mahal sehingga Anda hanya mampu membelinya, maka layak untuk membangun sistem yang memungkinkan orang menunggu giliran mereka untuk gagang pintu.

Dalam hal ini kenop pintu hanya referensi, jadi murah. Jika gagang pintu adalah objek aktual (dan mereka menggunakan kembali objek, bukan hanya referensi), maka itu mungkin cukup mahal untuk menjadi prudent global doorknob. Namun, ketika murah, itu dikenal sebagai cheapskate global doorknob.

Contoh Anda adalah cheapskatevarietas.

Ned Twigg
sumber
19

Perhatian utama di sini adalah konkurensi - Jika Foo digunakan oleh banyak utas, Anda memiliki masalah.

Lebih dari itu, itu konyol - variabel lokal mengelola siklus hidup mereka sendiri dengan sangat baik. Menggantinya dengan variabel instan yang perlu dibatalkan ketika mereka tidak lagi berguna adalah undangan untuk kesalahan.

Matthew Flynn
sumber
15

Ini adalah kasus spesifik "pelingkupan yang tidak tepat", dengan sisi "penggunaan kembali variabel".

ikegami
sumber
7

Itu bukan anti-pola. Anti-pola memiliki beberapa sifat yang membuatnya tampak seperti ide yang bagus, yang membuat orang sengaja melakukannya; mereka direncanakan sebagai pola dan kemudian menjadi sangat salah.

Itu juga yang membuat perdebatan tentang apakah sesuatu itu pola, anti-pola, atau pola yang umumnya salah diterapkan yang masih memiliki kegunaan di beberapa tempat.

Ini salah.

Untuk menambahkan sedikit lebih banyak.

Kode ini bersifat takhayul, atau paling tidak merupakan praktik pemujaan kargo.

Takhayul adalah sesuatu yang dilakukan tanpa pembenaran yang jelas. Mungkin terkait dengan sesuatu yang nyata, tetapi hubungannya tidak logis.

Praktik kultus kargo adalah salah satu di mana Anda mencoba menyalin sesuatu yang telah Anda pelajari dari sumber yang lebih luas, tetapi Anda sebenarnya menyalin artefak permukaan daripada prosesnya (yang dinamakan sebagai sekte di Papua Nugini yang akan menghasilkan radio kontrol pesawat keluar dari bambu dengan harapan membuat pesawat Jepang dan Amerika WWII kembali).

Dalam kedua kasus ini tidak ada kasus nyata yang dibuat.

Anti-pola adalah upaya peningkatan yang masuk akal, baik dalam jumlah kecil (percabangan ekstra untuk menangani kasus ekstra yang harus ditangani, yang mengarah ke kode spageti) atau dalam skala besar di mana Anda dengan sengaja menerapkan suatu pola baik yang didiskreditkan atau diperdebatkan (banyak yang akan menggambarkan lajang seperti itu, dengan beberapa mengecualikan hanya-menulis - misalnya penebangan objek atau read-only misalnya objek pengaturan konfigurasi - dan beberapa akan mengutuk bahkan yang) atau di mana Anda sedang memecahkan masalah yang salah (ketika .NET pertama kali dikeluarkan, MS merekomendasikan pola untuk berurusan dengan membuang ketika Anda memiliki bidang yang tidak dikelola dan bidang yang dapat dikelola - memang menangani situasi itu dengan sangat baik, tetapi masalah sebenarnya adalah Anda memiliki kedua jenis bidang di kelas yang sama).

Dengan demikian, anti-pola adalah sesuatu yang dengan sengaja dilakukan oleh orang pintar yang tahu bahasa, domain masalah, dan perpustakaan yang tersedia, yang masih memiliki (atau dianggap memiliki) sisi buruk yang menguasai sisi baiknya.

Karena tidak ada di antara kita yang mulai mengetahui bahasa tertentu, domain masalah, dan perpustakaan yang tersedia dengan baik, dan karena semua orang dapat melewatkan sesuatu saat mereka beralih dari satu solusi masuk akal ke solusi lain (mis. Mulai menyimpan sesuatu di bidang untuk penggunaan yang baik, lalu coba untuk refactor itu pergi tetapi tidak menyelesaikan pekerjaan, dan Anda akan berakhir dengan kode seperti dalam pertanyaan), dan karena kita semua kehilangan hal-hal dari waktu ke waktu dalam belajar, kita semua telah menciptakan beberapa kode kultus takhayul atau kargo pada suatu saat . Hal yang baik, adalah mereka sebenarnya lebih jelas untuk diidentifikasi dan dikoreksi daripada anti-pola. Anti-pola yang sebenarnya bisa dibilang bukan anti-pola, atau memiliki kualitas yang menarik, atau setidaknya memiliki beberapa cara untuk memikat seseorang ke dalamnya bahkan ketika diidentifikasi sebagai buruk (terlalu banyak dan terlalu sedikit lapisan keduanya buruk secara tidak kontroversial, tetapi menghindari satu mengarah ke yang lain).

Jon Hanna
sumber
1
Tetapi orang - orang berpikir bahwa ini adalah ide yang bagus, mungkin karena mereka berpikir itu adalah bentuk penggunaan kembali kode.
JSB ձոգչ
Pemula sering tampaknya berpikir bahwa mendeklarasikan dua variabel entah bagaimana membutuhkan ruang dua kali lipat, dan oleh karena itu lebih suka menggunakan kembali variabel. Ini menunjukkan kesalahpahaman dari model memori (free store vs stack, kemungkinan penggunaan kembali lokasi stack) dan optimisasi yang dapat dilakukan oleh semua kompiler. Saya berpendapat bahwa seorang pemula karena itu mungkin menganggap variabel menggunakan kembali ide yang baik, sehingga dapat diklasifikasikan sebagai antipattern.
Philipp
Itu menjadikannya sebuah takhayul, bukan antipattern.
Jon Hanna
3

(1) Saya akan mengatakan itu tidak baik.

Ini melakukan pemeliharaan rumah manual sehingga stack dapat dilakukan secara otomatis dengan variabel lokal.

Tidak ada manfaat kinerja karena "baru" disebut setiap metode pemanggilan.

EDIT: Jejak memori kelas lebih besar karena bilah penunjuk selalu mengambil memori sepanjang masa kelas. Jika bilah penunjuk adalah lokal, maka itu hanya akan menggunakan memori selama masa pemanggilan metode. Memori untuk pointer hanya setetes di lautan, tetapi itu masih setetes yang tidak perlu.

Bilah terlihat oleh metode lain di kelas. Metode yang tidak menggunakan bilah seharusnya tidak dapat melihatnya.

Kesalahan berbasis negara. Dalam kasus khusus ini, kesalahan basis negara seharusnya hanya terjadi dalam multi-threading karena "baru" dipanggil setiap kali.

(2) Saya tidak tahu apakah ada nama untuk itu karena itu sebenarnya beberapa masalah, bukan satu.
- Pembersihan rumah tangga otomatis ketika otomatis tersedia -
Status yang diperlukan - status
yang hidup lebih lama dari itu seumur hidup -
visibilitas atau ruang lingkup terlalu tinggi

mike30
sumber
2

Saya akan menyebutnya "Mengembara Xenophobe". Ia ingin dibiarkan sendiri tetapi akhirnya tidak tahu persis di mana atau apa itu. Jadi itu adalah hal yang buruk seperti yang dikatakan orang lain.

Insinyur Dunia
sumber
2

Melawan butir di sini tapi sementara saya tidak menganggap contoh yang diberikan sebagai gaya pengkodean yang baik seperti dalam bentuknya saat ini, polanya memang ada saat melakukan pengujian unit.

Ini cara standar dalam melakukan pengujian unit

public class BarTester
{
    private Bar bar;

    public Setup() { bar = new Bar(); }
    public Teardown() { bar = null; }

    public TestMethodA()
    {
        bar.A();        
    }

    public TestMethodB()
    {
        bar.B();
    }
}

hanyalah refactoring yang setara dengan kode OP ini

public class BarTester
{
    private Bar bar;

    public TestMethodA()
    {
        bar = new Bar();
        bar.A();
        bar = null;
    }

    public TestMethodB()
    {
        bar = new Bar();
        bar.B();
        bar = null;
    }
}

Saya tidak akan pernah menulis kode seperti yang diberikan oleh contoh OP, itu menjerit refactoring, tapi saya menganggap polanya tidak valid atau antipattern .

Letnan Keersmaekers
sumber
Dalam hal ini fixture instantiated secara independen untuk setiap tes, dan masalah yang terkait dengan variabel reuse atau concurrency tidak terjadi. Dalam kasus umum (tes unit luar) tidak diketahui apakah paling banyak satu metode dipanggil pada setiap objek.
Philipp
1
@ Pilip - Saya tidak membantah masalah penggunaan kembali atau konkurensi tetapi pertanyaan OP adalah tentang pola yang umum digunakan dalam pengujian unit. Fakta bahwa fixture dipakai untuk setiap tes adalah detail implementasi kerangka uji. Bahkan dalam pengujian unit, polanya hanya aman bila kerangka uji digunakan karena seharusnya digunakan. Alasan yang sama berlaku ketika menggunakan pola ini dalam kode produksi.
Lieven Keersmaekers
Tidak perlu diatur barke null, JUnit tetap membuat contoh pengujian baru untuk setiap metode pengujian.
Oberlies
2

Bau kode ini disebut sebagai Bidang Sementara dalam Refactoring oleh Beck / Fowler.

http://sourcemaking.com/refactoring/tentara-field

Diedit untuk menambahkan lebih banyak penjelasan dengan permintaan moderator: Anda dapat membaca lebih lanjut tentang bau kode di URL yang dirujuk di atas, atau dalam hard copy buku. Karena OP sedang mencari nama untuk bau antipattern / kode khusus ini, saya berpikir bahwa mengutip referensi yang tidak disebutkan sebelumnya dari sumber mapan yang berusia lebih dari 10 tahun akan sangat membantu, meskipun saya sepenuhnya menyadari bahwa saya terlambat ke mainkan pertanyaan ini, jadi sepertinya jawabannya tidak akan dipilih atau diterima.

Jika itu tidak terlalu pribadi promosi, saya juga referensi dan menjelaskan bau kode khusus ini dalam kursus Pluralsight saya yang akan datang, Refactoring Fundamentals, yang saya harapkan akan diterbitkan pada Agustus 2013.

Dengan cara menambahkan nilai lebih, mari kita bicara tentang cara memperbaiki masalah khusus ini. Ada beberapa opsi. Jika bidang benar-benar hanya digunakan oleh satu metode, maka itu dapat didemosiasikan ke variabel lokal. Namun, jika beberapa metode menggunakan bidang untuk berkomunikasi (sebagai pengganti parameter), maka ini adalah kasus klasik yang dijelaskan dalam Refactoring, dan dapat diperbaiki dengan mengganti bidang dengan parameter, atau jika metode yang bekerja dengan kode ini erat terkait, Kelas Ekstrak dapat digunakan untuk menarik metode dan bidang (s) ke dalam kelas mereka sendiri, di mana bidang selalu dalam keadaan valid (mungkin ditetapkan selama konstruksi) dan mewakili keadaan kelas baru ini. Jika pergi rute parameter, tetapi ada terlalu banyak nilai untuk dilewati, opsi lain adalah untuk memperkenalkan Obyek Parameter baru,

ssmith
sumber
Penting untuk dicatat bahwa Bidang Sementara sering diperlukan untuk refactoring Kelas Ekstrak. Tetapi seseorang yang mengetahui hal ini mungkin tidak akan memeriksa kode dalam kondisi perantara ini.
Oberlies
1

Saya akan mengatakan ketika Anda langsung ke sana, ini benar-benar kurangnya kohesi, dan saya mungkin memberinya nama "Kohesi Palsu". Saya akan membuat koin (atau jika saya mendapatkannya dari suatu tempat dan melupakannya, "pinjam") istilah ini karena kelas tampaknya kohesif karena metodenya tampaknya beroperasi pada bidang anggota. Namun, pada kenyataannya tidak, artinya kohesi yang tampak sebenarnya salah.

Mengenai apakah itu buruk atau tidak, saya akan mengatakannya dengan jelas, dan saya pikir Stephen C melakukan pekerjaan yang sangat baik untuk menjelaskan mengapa. Namun, apakah pantas disebut "anti-pola" atau tidak, saya pikir itu dan paradigma pengkodean lainnya dapat dilakukan dengan label, karena itu umumnya membuat komunikasi lebih mudah.

Erik Dietrich
sumber
1

Terlepas dari masalah yang telah disebutkan, menggunakan pendekatan ini di lingkungan tanpa pengumpulan sampah otomatis (misalnya C ++) akan menyebabkan kebocoran memori, karena objek sementara tidak dibebaskan setelah digunakan. (Meskipun ini mungkin agak sedikit dibuat-buat, ada orang di luar sana yang hanya akan mengubah 'null' menjadi 'NULL' dan senang bahwa kode dikompilasi.)

peterph
sumber
1

Bagi saya ini kelihatan seperti penerapan pola Flyweight yang mengerikan. Sayangnya saya kenal beberapa pengembang yang harus menggunakan "hal" yang sama beberapa kali dalam metode yang berbeda dan cukup menariknya ke bidang pribadi dalam upaya salah menghemat memori atau meningkatkan kinerja atau beberapa optimasi pseudo aneh lainnya. Dalam pikiran mereka, mereka mencoba untuk memecahkan masalah yang sama seperti Flyweight dimaksudkan untuk mengatasi hanya mereka melakukannya dalam situasi di mana sebenarnya bukan masalah yang perlu dipecahkan.

Evicatos
sumber
-1

Saya telah menemukan ini berkali-kali, biasanya ketika kode penyegaran yang sebelumnya ditulis oleh seseorang yang mengambil VBA For Dummies sebagai judul pertama mereka dan kemudian menulis sistem yang relatif besar dalam bahasa apa pun yang mereka temui.

Untuk mengatakan itu benar-benar praktik pemrograman yang buruk adalah benar, tetapi itu mungkin saja akibat dari tidak memiliki sumber daya internet & peer-review seperti yang kita semua nikmati hari ini yang mungkin telah memandu pengembang asli sepanjang jalur yang "lebih aman".

Itu tidak benar-benar layak dengan tag "antipattern" - tapi bagus untuk melihat saran tentang bagaimana OP dapat dikodekan ulang.

Lee James
sumber
1
Selamat datang di Programmer Stack Exchange, terima kasih telah memasukkan jawaban pertama Anda. Sepertinya Anda mengalami penurunan suara, mungkin karena alasan yang tercantum dalam programer FAQ.stackexchange.com/faq . FAQ memberikan saran bagus untuk membuat jawaban yang efektif (dan dipilih). Kadang-kadang orang cukup baik untuk menambahkan catatan tentang suara turun untuk menunjukkan apakah itu salah dalam beberapa hal, duplikat, pendapat tanpa bukti, atau aku juga yang mengulangi jawaban sebelumnya. Sebagian besar dari kita telah memilih turun, memilih, atau menghapus jawaban, jadi jangan khawatir. Itu hanya bagian dari memulai. Selamat datang.
PengembangDon