Dalam C dan C ++, metode apa yang dapat mencegah penggunaan tugas yang tidak disengaja (=) di mana kesetaraan (==) diperlukan?

15

Dalam C dan C ++, sangat mudah untuk menulis kode berikut dengan kesalahan serius.

char responseChar = getchar();
int confirmExit = 'y' == tolower(responseChar);
if (confirmExit = 1)
{
    exit(0);
}

Kesalahannya adalah bahwa pernyataan if seharusnya:

if (confirmExit == 1)

Sebagai kode, itu akan keluar setiap waktu, karena penugasan confirmExitvariabel terjadi, kemudian confirmExitdigunakan sebagai hasil dari ekspresi.

Adakah cara yang baik untuk mencegah kesalahan semacam ini?

c++ coding-standards Pengembang Don
sumber
39
Iya. Aktifkan peringatan kompiler. Perlakukan peringatan sebagai kesalahan dan itu bukan masalah.
Martin York
kemungkinan duplikat dari Bukankah "jika (0 == nilai) ..." lebih berbahaya daripada kebaikan?
Martin York
9
Dalam contoh yang diberikan, solusinya mudah. Anda menetapkan nilai boolean, sehingga menggunakannya sebagai boolean: if (confirmExit).
Amankan
4
Masalahnya adalah bahwa kesalahan itu dibuat oleh "desainer" bahasa C, ketika mereka memilih untuk menggunakan = untuk operator penugasan dan == untuk perbandingan kesetaraan. ALGOL menggunakan: =, karena mereka secara khusus ingin menggunakan = untuk perbandingan kesetaraan, dan PASCAL dan Ada mengikuti keputusan ALGOL. (Perlu dicatat bahwa, ketika DoD meminta entri berbasis C ke dalam DoD1 bake-off, yang akhirnya menghasilkan Ada, Bell Labs menolak, mengatakan bahwa "C tidak sekarang dan tidak akan pernah cukup kuat untuk DoD mission-critical perangkat lunak. "Seseorang berharap bahwa DoD dan kontraktor telah mendengarkan Bell Labs tentang ini.)
John R. Strohm
4
@ John, pilihan simbol bukan masalah, itu fakta bahwa penugasan juga merupakan ekspresi yang mengembalikan nilai yang ditetapkan, memungkinkan salah satu a = batau a == bdi dalam suatu kondisi.
Karl Bielefeldt

Jawaban:

60

Teknik terbaik adalah meningkatkan tingkat peringatan kompiler Anda. Ini kemudian akan memperingatkan Anda tentang penugasan potensial dalam kondisi jika.

Pastikan Anda mengkompilasi kode Anda dengan nol peringatan (yang seharusnya Anda lakukan). Jika Anda ingin menjadi jagoan maka atur kompiler Anda untuk memperlakukan peringatan sebagai kesalahan.

Menggunakan kondisi Yoda (meletakkan konstanta di sisi kiri) adalah teknik lain yang populer sekitar satu dekade lalu. Tetapi mereka membuat kode lebih sulit untuk dibaca (dan dengan demikian mempertahankan karena cara mereka membaca tidak wajar (kecuali Anda Yoda)) dan tidak memberikan manfaat lebih besar daripada meningkatkan tingkat peringatan (yang juga memiliki manfaat tambahan dari lebih banyak peringatan).

Peringatan adalah kesalahan yang benar-benar logis dalam kode dan harus diperbaiki.

Martin York
sumber
2
Jelas ikuti peringatan, bukan persyaratan Yoda - Anda bisa lupa untuk melakukan konstanta bersyarat terlebih dahulu semudah Anda lupa melakukannya ==.
Michael Kohne
8
Saya mendapat kejutan yang menyenangkan bahwa jawaban yang paling banyak dipilih untuk pertanyaan ini adalah 'ubah peringatan kompiler menjadi kesalahan'. Saya mengharapkan if (0 == ret)kengerian.
James
2
@ James: ... belum lagi itu tidak akan berhasil a == b!!
Emilio Garavaglia
6
@EmilioGaravaglia: Ada solusi mudah untuk itu: Cukup tulis 0==a && 0==b || 1==a && 1==b || 2==a && 2==b || ...(ulangi untuk semua nilai yang mungkin). Jangan lupa ...|| 22==a && 22==b || 23==a && 24==b || 25==a && 25==b ||kesalahan wajib ..., atau programmer pemeliharaan tidak akan bersenang-senang.
user281377
10

Anda selalu dapat melakukan sesuatu yang radikal seperti menguji perangkat lunak Anda. Saya bahkan tidak bermaksud pengujian unit otomatis, hanya tes yang dilakukan oleh setiap pengembang yang berpengalaman karena menjalankan kode barunya dua kali, sekali mengkonfirmasikan pintu keluar dan sekali tidak. Itulah alasan sebagian besar programmer menganggapnya sebagai tidak masalah.

Karl Bielefeldt
sumber
1
Mudah dilakukan ketika perilaku program Anda sepenuhnya deterministik.
James
3
Masalah khusus ini mungkin sulit untuk diuji. Saya telah melihat orang digigit rc=MethodThatRarelyFails(); if(rc = SUCCESS){lebih dari satu kali, terutama jika metode ini gagal hanya dalam kondisi yang sulit untuk diuji.
Gort the Robot
3
@ SvenvenBurnap: Itulah tujuan dari objek tiruan. Pengujian yang tepat mencakup pengujian mode kegagalan.
Jan Hudec
Ini jawaban yang benar.
Lightness Races dengan Monica
6

Cara tradisional untuk mencegah penggunaan tugas yang salah dalam ekspresi adalah dengan meletakkan konstanta di sebelah kiri dan variabel di sebelah kanan.

if (confirmExit = 1)  // Unsafe

if (1=confirmExit)    // Safe and detected at compile time.

Kompiler akan melaporkan kesalahan untuk penugasan ilegal ke konstanta yang mirip dengan yang berikut ini.

.\confirmExit\main.cpp:15: error: C2106: '=' : left operand must be l-value

Revisi jika kondisinya adalah:

  if (1==confirmExit)

Seperti yang ditunjukkan oleh komentar di bawah, ini dianggap oleh banyak orang sebagai metode yang tidak pantas.

Pengembang Don
sumber
8
Juga dikenal sebagai Yoda conditional: wiert.me/2010/05/25/… dan stackoverflow.com/questions/8591182/… dan programmer.stackexchange.com/questions/16908/…
Martin York
13
Perlu dicatat bahwa melakukan hal-hal seperti ini akan membuat Anda cukup tidak populer.
riwalk
11
Tolong jangan rekomendasikan praktik ini.
James
5
Ini juga tidak berfungsi jika Anda perlu membandingkan dua variabel satu sama lain.
dan04
Beberapa bahasa sebenarnya memungkinkan penetapan 1 ke nilai baru (ya, saya tahu Q adalah pertarungan c / c ++)
Matsemann
4

Saya setuju dengan semua orang yang mengatakan "peringatan kompiler" tetapi saya ingin menambahkan teknik lain: Ulasan kode. Jika Anda memiliki kebijakan untuk meninjau semua kode yang dikomit, lebih disukai sebelum dikomit, maka kemungkinan hal semacam ini akan ditangkap selama peninjauan.

MatrixFrog
sumber
Saya tidak setuju. Terutama = bukannya == dapat dengan mudah menyelinap dengan meninjau kode.
Simon
2

Pertama, menaikkan level peringatan Anda tidak pernah sakit.

Jika Anda tidak ingin persyaratan Anda untuk menguji hasil penugasan dalam pernyataan if itu sendiri, maka setelah bekerja dengan banyak programmer C dan C ++ selama bertahun-tahun, dan tidak pernah mendengar bahwa membandingkan konstanta pertama if(1 == val)adalah hal yang buruk, Anda bisa mencoba konstruk itu.

Jika pemimpin proyek Anda menyetujui Anda melakukan ini, jangan khawatir tentang apa yang dipikirkan orang lain. Bukti nyata adalah apakah Anda atau orang lain dapat memahami kode Anda berbulan-bulan dan bertahun-tahun dari sekarang.

Namun, jika Anda bermaksud menguji hasil dari suatu tugas, maka menggunakan peringatan yang lebih tinggi mungkin [mungkin akan] menangkap tugas tersebut menjadi konstan.

octopusgrabbus
sumber
Saya mengangkat alis skeptis pada programmer non-pemula yang melihat Yoda bersyarat dan tidak segera memahaminya. Ini hanya flip, tidak sulit untuk dibaca, dan tentu saja tidak seburuk yang diklaim beberapa komentar.
underscore_d
@underscore_d Di sebagian besar atasan saya, tugas dalam sebuah kondisional tidak disukai. Pemikirannya adalah lebih baik untuk memisahkan tugas dari persyaratan. Alasan untuk menjadi lebih jelas, bahkan dengan mengorbankan baris kode lain adalah faktor rekayasa yang berkelanjutan. Saya bekerja di tempat-tempat yang memiliki basis kode besar dan banyak pemeliharaan yang ditumpuk. Saya sepenuhnya memahami seseorang mungkin ingin memberikan nilai, dan bercabang pada kondisi yang dihasilkan dari penugasan. Saya melihat itu dilakukan lebih sering di Perl, dan, jika maksudnya jelas, saya akan mengikuti pola desain penulis.
octopusgrabbus
Saya sedang memikirkan kondisional Yoda saja (seperti demo Anda di sini), bukan dengan tugas (seperti dalam OP). Saya tidak keberatan yang pertama tetapi tidak terlalu suka yang kedua juga. Satu-satunya bentuk yang saya sengaja gunakan adalah if ( auto myPtr = dynamic_cast<some_ptr>(testPtr) ) {karena menghindari menjaga ruang nullptrlingkup yang tidak berguna jika para pemain gagal - yang mungkin alasan C ++ memiliki kemampuan terbatas ini untuk menetapkan dalam kondisi. Untuk selebihnya, ya, sebuah definisi harus mendapatkan garisnya sendiri, saya katakan - jauh lebih mudah untuk dilihat sekilas dan kurang rentan terhadap berbagai macam slip-of-the-mind.
underscore_d
@underscore_d Jawaban yang diedit berdasarkan komentar Anda. Poin yang bagus.
octopusgrabbus
1

Terlambat ke pesta seperti biasanya, tetapi Analisis Kode Statis adalah kuncinya di sini

Sebagian besar IDE sekarang menyediakan SCA melebihi dan di atas pemeriksaan sintaksis dari kompiler, dan alat-alat lain tersedia, termasuk yang mengimplementasikan pedoman MISRA (*) dan / atau CERT-C.

Deklarasi: Saya bagian dari kelompok kerja MISRA C, tetapi saya memposting dalam kapasitas pribadi. Saya juga independen dari vendor alat apa pun

Andrew
sumber
-2

Cukup gunakan tugas sebelah kiri, peringatan kompiler dapat membantu tetapi Anda harus memastikan Anda mendapatkan tingkat yang tepat jika tidak, Anda akan dibanjiri dengan peringatan tidak berguna atau tidak akan diberitahu yang ingin Anda lihat.

Llama terbalik
sumber
4
Anda akan terkejut melihat betapa sedikit peringatan tidak berguna yang dihasilkan oleh kompiler modern. Anda mungkin tidak menganggap peringatan itu penting, tetapi dalam kebanyakan kasus Anda salah.
Kristof Provost
Lihat buku, "Menulis Kode Padat" amazon.com/Writing-Solid-Microsoft-Programming-Series/dp/… . Ini dimulai dengan diskusi hebat tentang kompiler dan berapa banyak manfaat yang bisa kita dapatkan dari pesan peringatan dan bahkan analisis statis yang lebih luas.
PengembangDon
@ KristofProvost Saya sudah berhasil mendapatkan visual studio untuk memberi saya 10 salinan peringatan yang sama persis dari baris kode yang sama, maka ketika masalah ini 'diperbaiki' itu menghasilkan 10 peringatan identik tentang baris kode yang sama yang mengatakan bahwa metode asli lebih baik.
Terbalik Llama