Boolean mengembalikan set.add () jika bersyarat?

15

Add operator dari kelas set mengembalikan boolean yang benar jika elemen (yang akan ditambahkan) belum ada di sana, dan false jika tidak. Sedang menulis

if (set.add(entry)) {
    //do some more stuff
}

dianggap gaya yang baik dalam hal menulis kode bersih? Saya ingin tahu karena Anda melakukan dua hal sekaligus. 1) menambahkan elemen dan 2) memeriksa apakah elemen tersebut ada.

Andreas Braun
sumber
6
Anda sedang berbicara tentang standar java.util.Set, yang mengembalikan true addketika elemen itu belum ada di sana, kan?
user2357112 mendukung Monica
1
Saya biasanya akan mempertimbangkan tes sebaliknya:if (!set.add(entry)) {// entry already present, possibly a case you want to handle}
njzk2
Apakah ada yang salah dengan melakukan dua hal sekaligus?
user207421
@ user2357112 ya, itu benar.
Andreas Braun

Jawaban:

19

Ya itu.

Titik biasa dari operasi mengembalikan nilai Boolean adalah bahwa Anda dapat menggunakannya untuk membuat keputusan, yaitu dalam ifkonstruk. Satu-satunya cara Anda dapat menyadari potensi ini adalah dengan menyimpan nilai kembali dalam variabel dan kemudian segera menggunakannya kembali dalam if, yang hanya konyol dan tentu saja tidak dengan cara apa pun lebih baik daripada menulis if(operation()) { ... }. Jadi silakan saja dan lakukan itu, kami tidak akan menghakimi Anda (untuk itu).

Kilian Foth
sumber
Terima kasih atas jawaban anda. Sebagai @Niels vanReijmersdal menunjukkan dalam jawabannya melakukan ini melanggar perintah dan permintaan pemisahan, bukan? Tidakkah menurut Anda itu penting?
Andreas Braun
4
@AndreasBraun pribadi saya pikir pemisahan permintaan perintah bodoh. Seringkali logis untuk satu metode untuk melakukan suatu tindakan dan mengembalikan nilai yang terkait dengan tindakan itu. Menegakkan pemisahan buatan antara kedua mengarah ke kode verbose yang tidak perlu.
1
@ dan1111: memang. Selanjutnya, "pemisahan perintah dan permintaan" mengarah ke "centang lalu bertindak" dan pola anti serupa, yang dapat pecah dalam konteks multi-utas. Agak aneh mengiklankan pemisahan seperti itu, ketika seluruh dunia sedang mencoba untuk membangun operasi atom menyatu untuk solusi SMP yang kuat dan efisien pada saat yang sama ...
Holger
2
@ Jörg W Mittag: halaman 759 tentang apa? Karena operasi atom secara intrinsik kebal terhadap pola anti-centang "check then act", tidak ada gunanya membaginya hanya dengan membaca "seluruh bab" dari apa pun untuk mencari tahu, bagaimana membuat thread konstruksinya aman lagi. Karena Anda tidak menyebutkan argumen yang sebenarnya, saya tidak punya "keberatan khusus untuk argumen itu".
Holger
1
@ Jörg W Mittag: Ya, saya sedang berbicara tentang jawaban di halaman ini, mengecilkan penggunaan Set.addsebagai operasi tunggal tanpa menyebutkan alternatif. Jika operasi yang dimaksudkan adalah untuk menambahkan elemen tunggal dan mencari tahu apakah telah ditambahkan, tidak perlu alternatif yang lebih kuat yang masih mempersulit operasi tanpa manfaat. Saya harap Anda sadar bahwa Set.addini adalah metode JRE bawaan yang dapat digunakan tanpa mempelajari buku> 700 halaman terlebih dahulu.
Holger
12

Saya akan mengatakan itu tidak sebersih mungkin, karena memaksa pengelola untuk sudah tahu atau mencari tahu apa nilai kembaliannya. Apakah ini berarti nilai sudah ada, tidak sudah ada, berhasil dimasukkan? Jika Anda tidak sering menggunakannya, Anda tidak akan tahu, dan bahkan jika Anda menggunakannya, itu adalah beban mental yang jauh lebih besar.

Saya lebih suka yang berikut ini:

boolean added = set.add(entry);

if (added) {
    //do some more stuff
}

Ya, sedikit lebih banyak verbose, tetapi kompiler harus menghasilkan bytecode yang persis sama, dan bahkan orang-orang yang tidak menggunakan set Java bertahun-tahun dapat mengikuti logika tanpa melihat apa-apa.

Karl Bielefeldt
sumber
2
addmengembalikan true jika elemen belum ada di sana, tidak jika itu ada. (Kata-kata pertanyaan itu menyesatkan.)
user2357112 mendukung Monica
9
Mencari metode adalah lebih mudah daripada mencoba memahami maksud pengembang asli untuk variabel yang berlebihan yang dinyatakan di luar lingkup tempat metode itu digunakan. Di semua IDE Java modern, pengembang dapat mengarahkan pointer mouse mereka ke suatu metode dan langsung mengakses dokumentasinya. Pengembang yang baik melakukan hal ini secara konstan ketika bekerja dengan API yang tidak dikenal.
Kevin Krumwiede
9
Saya kedua @ Holger. Jika Anda tidak tahu Set.add, Anda tidak berpengalaman dan Anda harus mencari metode ini untuk mempelajarinya dan menjadi lebih berpengalaman.
Pasang kembali Monica
7
notAlreadyPresentbukan kata-kata terbaik. Saya akan menggunakan added, dan berharap pembaca tahu mengapa suatu nilai tidak akan ditambahkan ke Set.
njzk2
3
@JustinTime: Menggunakan komentar untuk menggambarkan kode apa yang secara luas dianggap praktik buruk. Kode Anda harus menggambarkan diri sendiri. Dalam ulasan kode, saya akan menandai contoh Anda sebagai tidak dapat diterima.
BlueRaja - Danny Pflughoeft
8

Jika true berarti sukses, maka itu kode yang baik dan jelas.

Ada konvensi luas bahwa suatu fungsi atau metode mengembalikan true (atau sesuatu yang mengevaluasi true) pada kesuksesan. Selama kode Anda mengikuti itu, saya pikir menempatkan metode dalam kondisi baik-baik saja.

Kode seperti ini tidak perlu berantakan dalam pandangan saya:

boolean frobulate_succeeded = thing.frobulate();

if (frobulate_succeeded) {
    ...
}

Rasanya seperti Anda mengulangi sendiri.

Namun, pertanyaannya tidak jelas tentang arti nilai kembali. Anda mengatakan "boolean yang menunjukkan apakah elemen yang ditambahkan sudah ada", yang mungkin menyiratkan bahwa true berarti elemen itu ada (dan menambahkan tidak terjadi). Jika demikian, saya idealnya mengubah perilaku pengembalian metode menjadi lebih konvensional. Jika itu tidak memungkinkan, saya akan menambahkan variabel perantara tambahan yang memungkinkan Anda untuk dengan jelas memberi label hasil pengembalian dalam kode Anda (seperti yang disarankan oleh orang lain).


sumber
Apa arti sukses dalam konteks menambahkan item ke set? Tidak melempar pengecualian berarti sukses; true vs false berarti sesuatu yang lebih spesifik dalam konteks ini.
Pasang kembali Monica
@ Solomonoff'sSecret Dalam kasus ini, pengecualian berarti set menolak untuk menambahkan elemen, falseberarti elemen tidak ditambahkan karena sudah terkandung, dan trueberarti elemen belum terkandung. Seperti add()menambahkan elemen ke set jika set belum mengandungnya, truekarena itu berarti elemen berhasil ditambahkan ke set.
Justin Time - Pasang kembali Monica
@JustinTime Anda menambahkan penilaian nilai Anda sendiri ke dua kasus. Interpretasi lain adalah kesuksesan itu adalah bahwa item tersebut ada di set ketika metode kembali. Jika item sudah di set, addberhasil tanpa harus melakukan apa pun. Jika item belum di set, addberhasil dengan menambahkan item ke set. Penafsiran mana yang benar adalah arbitrer. Definisi sukses yang kurang sembarangan adalah definisi dari bahasa: metode berhasil jika kembali secara normal.
Pasang kembali Monica
1
Definisi keberhasilan yang paling tidak sewenang-wenang adalah bahwa metode tersebut berhasil melakukan tugas yang harus dilakukan. Jika saya memiliki metode firstLessThanSecond(int l, int r), dan itu mengembalikan truejika l > ratau falsejika l <= r, maka metode itu tidak berhasil, meskipun itu kembali secara normal.
Justin Time - Pasang kembali Monica
1
@JustinTime addadalah singkatan dari kontrak. Dokumentasi dimulai dengan "Menambahkan elemen yang ditentukan ke set ini jika belum ada", yang puas apakah item sudah dalam set atau tidak. Anda bebas menginterpretasikan nilai pengembalian sesuai keinginan Anda, tetapi pada akhirnya itu hanya interpretasi Anda. Dan dalam contoh kedua Anda, metode mengevaluasi apakah suatu kondisi benar. Jika kondisinya salah, metode tersebut pasti tidak gagal, karena telah berhasil mengevaluasi kondisi tersebut.
Pasang kembali Monica
2

Saya akan mengatakan itu sangat seperti C. Sebagian besar waktu saya lebih suka memiliki variabel yang dinamai secara deskriptif untuk hasil mutasi, dan tidak ada mutasi yang terjadi dalam suatu ifkondisi.

Kompiler akan menghilangkan variabel ini jika langsung digunakan kembali. Manusia akan lebih mudah membaca sumbernya; bagi saya, ini lebih penting.

Jika seseorang harus memperpanjang kondisi dengan menambahkan and/ orklausa ke kondisi if, mereka mungkin akhirnya tidak menelepon .add()dalam kasus-kasus tertentu karena evaluasi hubungan pendek. Kecuali jika korsleting diantisipasi secara khusus, ini mungkin berakhir sebagai bug.

9000
sumber
Jika saya menulis if (set.contains(entry)){set.add(entry); //do more stuff}apakah itu juga dihilangkan oleh kompiler?
Andreas Braun
1
@AndreasBraun: Saya rasa tidak; kompiler tidak tahu tentang tautan semantik antara containsdan add. Selain itu, ini berfungsi ganda dalam memandang entryset; ini mungkin memainkan peran untuk set yang sangat besar dan loop yang sangat ringan.
9000
1
Jadi saya kira Anda lebih suka tidak menulis if (set.contains(entry)){set.add(entry); //do more stuff}tetapi pergi dengan misalnya jawaban Karl Bielefeldt?
Andreas Braun
@AndreasBraun: tepat (diputuskan jawaban itu).
9000
1
poin yang bagus. Mutasi pada ifs adalah rumit, karena dev masa depan mungkin memiliki kondisi lain dengan korsleting.
njzk2
0

Kode Anda tampaknya memecah Command Query Separation . Ini dibahas dalam buku Kode Bersih dan video Function Structure . Jadi dari perspektif Clean Code saya pikir itu tidak dianggap gaya yang baik.

“Kode bersih itu sederhana dan langsung. Kode bersih berbunyi seperti prosa yang ditulis dengan baik. Kode bersih tidak pernah mengaburkan niat desainer melainkan penuh dengan abstraksi renyah dan garis kontrol langsung.

- Grady Booch penulis Object Oriented Analysis and Design with Applications ”

Bagi saya maksud kode Anda tidak jelas. Apakah jika keduanya dieksekusi ketika entri ditambahkan berhasil, atau juga ketika sudah ada? Apa yang add()kembali? barang itu? Kode kesalahan?

Niels van Reijmersdal
sumber
2
Ya, itulah yang saya pikirkan juga. Tapi saya juga berpikir @Kilian Foth ada benarnya. Jika saya ingin memisahkan permintaan dan perintah saya harus menulis if (set.contains(entry)){set.add(entry); //do more stuff}yang sepertinya konyol. Apa pendapat Anda tentang ini?
Andreas Braun
Saya tidak tahu domain dan konteks kode ini untuk menyarankan nama yang baik, tetapi saya akan membungkusnya dalam fungsi dengan maksud yang jelas. Sebagai contoh: doesEntryExistOrCreateEntry (entri), ini bisa berisi logika Anda berisi () + add () dan mengembalikan boolean. Pertanyaan serupa di sini: softwareengineering.stackexchange.com/questions/149708/... yang membahas praktik ini di luar kode bersih.
Niels van Reijmersdal
6
Saya pikir aturan pemisahan kueri perintah adalah bodoh, terutama ketika diterapkan pada kasus seperti ini, di mana metode keduanya melakukan suatu tindakan dan mengembalikan keadaan keberhasilan tindakan. Tetapi juga, Anda tidak menjelaskan apa aturannya atau memberikan banyak cara pembenaran untuk itu. Turun karena alasan ini.
1
"Apa yang ditambahkan () kembali?" Saya berharap ada pengembang java yang melakukan review kode untuk mengetahui Collectionapi.
njzk2
3
Setuju dengan @ dan1111. Ini terutama bodoh karena itu adalah hal-hal yang membuat tanah subur untuk kondisi ras.
Paul Draper