Hal-hal mana yang langsung membunyikan lonceng alarm ketika melihat kode? [Tutup]

Saya menghadiri acara pengerjaan perangkat lunak beberapa minggu yang lalu dan salah satu komentar yang dibuat adalah "Saya yakin kita semua mengenali kode buruk ketika kita melihatnya" dan semua orang mengangguk dengan bijak tanpa diskusi lebih lanjut.

Hal semacam ini selalu membuat saya khawatir karena ada disangkal bahwa semua orang berpikir mereka adalah pengemudi di atas rata-rata. Meskipun saya pikir saya bisa mengenali kode buruk, saya ingin belajar lebih banyak tentang apa yang orang lain anggap sebagai kode bau karena jarang dibahas secara rinci di blog orang dan hanya di segelintir buku. Khususnya saya pikir akan menarik untuk mendengar tentang apa pun yang merupakan bau kode dalam satu bahasa tetapi bukan yang lain.

Saya akan mulai dengan yang mudah:

Kode dalam kontrol sumber yang memiliki proporsi tinggi kode komentar - mengapa ada di sana? apakah itu dimaksudkan untuk dihapus? Apakah itu pekerjaan setengah jadi? mungkin itu seharusnya tidak dikomentari dan hanya dilakukan ketika seseorang sedang menguji sesuatu? Secara pribadi saya menemukan hal semacam ini sangat mengganggu bahkan jika itu hanya garis aneh di sana-sini, tetapi ketika Anda melihat blok besar diselingi dengan sisa kode itu benar-benar tidak dapat diterima. Ini juga biasanya merupakan indikasi bahwa sisa kode kemungkinan memiliki kualitas yang meragukan juga.

/* Fuck this error */

Biasanya ditemukan di dalam try..catchblok omong kosong , itu cenderung menarik perhatian saya. Hampir sama baiknya /* Not sure what this does, but removing it breaks the build */.

Beberapa hal lagi:

• Beberapa ifpernyataan kompleks bersarang
• Blok coba-tangkap yang digunakan untuk menentukan aliran logika secara teratur
• Fungsi dengan nama generik process, data, change, rework,modify
• Enam atau tujuh gaya penahan yang berbeda dalam 100 baris

Yang baru saya temukan:

/* Stupid database */
$conn = null;
while(!$conn) {
    $conn = mysql_connect("localhost", "root", "[pass removed]");
}
/* Finally! */
echo("Connected successfully.");

Benar, karena harus memaksa koneksi MySQL Anda adalah cara yang tepat untuk melakukan sesuatu. Ternyata database itu mengalami masalah dengan jumlah koneksi sehingga mereka terus waktu. Alih-alih men-debug ini, mereka hanya mencoba lagi dan lagi sampai berhasil.

Bendera merah utama bagi saya adalah blok kode duplikat, karena itu menunjukkan bahwa orang tersebut tidak memahami dasar-dasar pemrograman atau terlalu takut untuk membuat perubahan yang tepat ke basis kode yang ada.

Saya dulu juga menghitung kurangnya komentar sebagai bendera merah, tetapi setelah baru-baru ini bekerja pada banyak kode yang sangat baik tanpa komentar saya sudah mereda kembali pada itu.

Kode yang mencoba menunjukkan betapa pandai programmer adalah, terlepas dari kenyataan bahwa itu tidak menambah nilai nyata:

x ^= y ^= x ^= y;

• 20.000 fungsi garis (berlebihan). Setiap fungsi yang membutuhkan lebih dari beberapa layar perlu di-factoring ulang.

• Sepanjang baris yang sama, file kelas yang sepertinya berlangsung selamanya. Mungkin ada beberapa konsep yang dapat diabstraksi menjadi kelas-kelas yang akan menjernihkan tujuan dan fungsi kelas asli, dan mungkin di mana ia digunakan, kecuali mereka semua adalah metode internal.

• variabel non-deskriptif, non-trivial, atau terlalu banyak variabel non-deskriptif sepele. Ini membuat menyimpulkan apa yang sebenarnya terjadi teka-teki.

{ Let it Leak, people have good enough computers to cope these days }

Yang lebih parah adalah itu dari perpustakaan komersial!

Komentar yang sangat verbose bahwa jika ada kompiler bahasa Inggris, itu akan mengkompilasi dan berjalan dengan sempurna, namun tidak menggambarkan apa pun yang kode tidak.

//Copy the value of y to temp.
temp = y;
//Copy the value of x to y.
y = x;
//Copy the value of temp to x.
x = temp;

Juga, komentar pada kode yang bisa dihapuskan seandainya kode tersebut mematuhi beberapa pedoman dasar:

//Set the age of the user to 13.
a = 13;

Kode yang menghasilkan peringatan saat dikompilasi.

Berfungsi dengan angka dalam nama alih-alih memiliki nama deskriptif , seperti:

void doSomething()
{
}

void doSomething2()
{
}

Tolong, buat nama fungsi berarti sesuatu! Jika doSomething dan doSomething2 melakukan hal serupa, gunakan nama fungsi yang membedakan perbedaan. Jika doSomething2 adalah pelarian fungsionalitas dari doSomething, beri nama untuk fungsionalitasnya.

Angka Ajaib atau String Ajaib.

   if (accountBalance>200) { sendInvoice(...); }

   salesPrice *= 0.9;   //apply discount    

   if (invoiceStatus=="Overdue") { reportToCreditAgency(); }

• Mungkin bukan yang terburuk tetapi jelas menunjukkan tingkat pelaksana:

  if(something == true) 

• Jika suatu bahasa memiliki for for atau iterator construct, maka menggunakan while sementara juga menunjukkan tingkat pemahaman pelaksana bahasa:

  count = 0; 
  while(count < items.size()){
     do stuff
     count ++; 
  }
  
  for(i = 0; i < items.size(); i++){
    do stuff 
  }
  //Sure this is not terrible but use the language the way it was meant to be used.
  

• Ejaan / tata bahasa yang buruk dalam dokumentasi / komentar memakan saya hampir sebanyak kode itu sendiri. Alasan untuk ini adalah karena kode dimaksudkan untuk dibaca manusia dan mesin dijalankan. Inilah sebabnya kami menggunakan bahasa tingkat tinggi, jika dokumentasi Anda sulit untuk menembusnya membuat saya lebih dulu membentuk opini negatif tentang basis kode tanpa melihatnya.

Yang saya perhatikan segera adalah frekuensi blok kode yang sangat bersarang (jika, sementara, dll). Jika kode sering lebih dari dua atau tiga level, itu pertanda masalah desain / logika. Dan jika kedalamannya mencapai 8 sarang, lebih baik ada alasan kuat untuk tidak putus.

Saat menilai program siswa, kadang-kadang aku bisa mengatakannya dengan gaya "blink". Ini adalah petunjuk instan:

• Pemformatan buruk atau tidak konsisten
• Lebih dari dua baris kosong berturut-turut
• Konvensi pemberian nama yang tidak standar atau tidak konsisten
• Kode berulang, semakin kata demi kata berulang, semakin kuat peringatan
• Apa yang seharusnya menjadi potongan kode sederhana terlalu rumit (misalnya, memeriksa argumen yang diteruskan ke utama dengan cara yang berbelit-belit)

Jarang kesan pertama saya salah, dan lonceng peringatan ini benar sekitar 95% . Untuk satu pengecualian, seorang siswa baru ke bahasa itu menggunakan gaya dari bahasa pemrograman yang berbeda. Menggali dan membaca ulang gaya mereka dalam idiom bahasa lain menghilangkan bel alarm untuk saya, dan siswa kemudian mendapat pujian penuh. Tetapi pengecualian seperti itu jarang terjadi.

Saat mempertimbangkan kode lebih lanjut, ini adalah peringatan saya yang lain:

• Kehadiran banyak kelas Java yang hanya "struct" untuk menyimpan data. Tidak masalah jika bidangnya publik atau pribadi dan menggunakan getter / setter, itu masih mungkin bukan bagian dari desain yang dipikirkan dengan matang.
• Kelas yang memiliki nama yang buruk, seperti hanya menjadi namespace dan tidak ada kohesi yang nyata dalam kode
• Referensi pola desain yang bahkan tidak digunakan dalam kode
• Penangan pengecualian kosong tanpa penjelasan
• Ketika saya menarik kode di Eclipse, ratusan "peringatan" garis kuning kode, sebagian besar karena impor atau variabel yang tidak digunakan

Dalam hal gaya, saya biasanya tidak suka melihat:

• Komentar Javadoc yang hanya mengulangi kode

Ini hanya petunjuk kode yang buruk. Kadang-kadang apa yang tampak seperti kode buruk sebenarnya tidak, karena Anda tidak tahu niat programmer. Misalnya, mungkin ada alasan bagus bahwa sesuatu tampak terlalu rumit - mungkin ada pertimbangan lain yang berperan.

Favorit pribadi / kesayangan hewan peliharaan: Nama yang dihasilkan IDE yang dapat dikirim. Jika TextBox1 adalah variabel utama dan penting dalam sistem Anda, Anda punya hal lain yang datang ulasan kode.

Variabel yang sama sekali tidak digunakan , terutama ketika variabel tersebut memiliki nama yang mirip dengan nama variabel yang digunakan.

Banyak orang menyebutkan:

//TODO: [something not implemented]

Sementara saya berharap hal-hal itu dilaksanakan, setidaknya mereka membuat catatan. Yang saya pikir lebih buruk adalah:

//TODO: [something that is already implemented]

Todo tidak berharga dan membingungkan jika Anda tidak pernah repot-repot menghapusnya!

Metode yang mengharuskan saya untuk menggulir ke bawah untuk membaca semuanya.

Konjungsi dalam nama metode:

public void addEmployeeAndUpdatePayrate(...) 


public int getSalaryOrHourlyPay(int Employee) ....

Klarifikasi: Alasan mengapa alarm berbunyi adalah karena ini mengindikasikan metode yang kemungkinan melanggar prinsip tanggung jawab tunggal .

Menghubungkan kode sumber yang jelas GPL ke dalam program komersial, sumber tertutup.

Tidak hanya membuat masalah hukum langsung, tetapi dalam pengalaman saya, biasanya menunjukkan baik kecerobohan atau tidak peduli yang tercermin di tempat lain dalam kode juga.

Bahasa agnostik:

• TODO: not implemented
• int function(...) { return -1; } (sama dengan "tidak diterapkan")
• Melempar pengecualian karena alasan yang tidak biasa.
• Penyalahgunaan atau penggunaan yang tidak konsisten 0, -1atau nullsebagai nilai pengembalian yang luar biasa.
• Pernyataan tanpa komentar meyakinkan yang mengatakan mengapa itu tidak boleh gagal.

Khusus bahasa (C ++):

• C ++ Macro dalam huruf kecil.
• Variabel C ++ Statis atau Global.
• Variabel tidak diinisialisasi atau tidak digunakan.
• Apa pun array newyang tampaknya tidak aman RAII.
• Setiap penggunaan array atau pointer yang tampaknya tidak aman-batas. Ini termasuk printf.
• Setiap penggunaan bagian array yang tidak diinisialisasi.

Khusus Microsoft C ++:

• Setiap nama pengidentifikasi yang berbenturan dengan makro sudah ditentukan oleh salah satu file header Microsoft SDK.
• Secara umum, setiap penggunaan Win32 API adalah sumber lonceng alarm yang besar. Selalu minta MSDN terbuka, dan cari argumen / definisi kembali nilai setiap kali ragu. (Diedit)

Khusus C ++ / OOP:

• Warisan implementasi (kelas konkret) di mana kelas induk memiliki metode virtual dan non-virtual, tanpa perbedaan konseptual yang jelas antara apa yang seharusnya / tidak boleh virtual.

Gaya lekukan yang aneh.

Ada beberapa gaya yang sangat populer, dan orang-orang akan membawa perdebatan itu ke kubur. Tetapi kadang-kadang saya melihat seseorang menggunakan gaya lekukan yang benar-benar langka, atau bahkan buatan sendiri. Ini adalah tanda bahwa mereka mungkin belum mengkodekan dengan orang lain selain diri mereka sendiri.

Menggunakan banyak blok teks daripada enum atau variabel yang didefinisikan secara global.

Tidak baik:

if (itemType == "Student") { ... }

Lebih baik:

private enum itemTypeEnum {
  Student,
  Teacher
}
if (itemType == itemTypeEnum.Student.ToString()) { ... }

Terbaik:

private itemTypeEnum itemType;
...
if (itemType == itemTypeEnum.Student) { ... }

Parameter yang diketik dengan lemah atau mengembalikan nilai pada metode.

public DataTable GetEmployees() {...}

public DateTime getHireDate(DataTable EmployeeInfo) {...}

public void FireEmployee(Object EmployeeObjectOrEmployeeID) {...}

• Beberapa operator ternary dirangkai, jadi alih-alih menyerupai if...elseblok, itu menjadi if...else if...[...]...elseblok
• Nama variabel panjang tanpa garis bawah atau CamelCasing. Contoh dari beberapa kode yang saya ambil:$lesseeloginaccountservice
• Ratusan baris kode dalam file, dengan sedikit atau tidak ada komentar, dan kode menjadi sangat tidak jelas
• ifPernyataan yang terlalu rumit . Contoh dari kode: if (!($lessee_obj instanceof Lessee && $lessee_obj != NULL))yang saya gunakan untuk chompedif ($lessee_obj == null)

Kode bau: tidak mengikuti praktik terbaik

Hal semacam ini selalu membuat saya khawatir karena ada disangkal bahwa semua orang berpikir mereka adalah pengemudi di atas rata-rata.

Inilah kilasan berita untuk Anda: 50% dari populasi dunia di bawah rata-rata intelijen. Ok, jadi beberapa orang akan memiliki kecerdasan rata-rata, tapi jangan pilih-pilih Juga, salah satu sisi yang mempengaruhi kebodohan adalah Anda tidak bisa mengenali kebodohan Anda sendiri! Hal-hal tidak terlihat begitu baik jika Anda menggabungkan pernyataan ini.

Hal-hal mana yang langsung membunyikan lonceng alarm ketika melihat kode?

Banyak hal baik telah disebutkan, dan secara umum tampaknya tidak mengikuti praktik terbaik adalah bau kode.

Praktik terbaik biasanya tidak ditemukan secara acak, dan sering ada karena suatu alasan. Sering kali itu bisa subjektif, tetapi dalam pengalaman saya kebanyakan dibenarkan. Mengikuti praktik terbaik seharusnya tidak menjadi masalah, dan jika Anda bertanya-tanya mengapa hal itu terjadi, teliti daripada mengabaikan dan / atau mengeluh tentang hal itu - mungkin itu dibenarkan, mungkin tidak.

Salah satu contoh praktik terbaik mungkin menggunakan ikal dengan setiap blok jika, meskipun hanya berisi satu baris:

if (something) {
    // whatever
}

Anda mungkin tidak berpikir itu perlu, tetapi saya baru-baru ini membaca bahwa itu adalah sumber utama bug. Selalu menggunakan tanda kurung juga telah dibahas pada Stack Overflow , dan memeriksa bahwa jika pernyataan memiliki tanda kurung juga merupakan aturan dalam PMD , penganalisa kode statis untuk Java.

Ingat: "Karena ini praktik terbaik" tidak pernah merupakan jawaban yang dapat diterima untuk pertanyaan "mengapa Anda melakukan ini?" Jika Anda tidak dapat mengartikulasikan mengapa sesuatu itu praktik terbaik, maka itu bukan praktik terbaik, itu takhayul.

Komentar yang mengatakan "ini karena desain dari subsistem froz benar-benar borked."

Itu berlangsung lebih dari satu paragraf penuh.

Mereka menjelaskan bahwa refactor berikut perlu terjadi.

Tetapi tidak melakukannya.

Sekarang, mereka mungkin telah diberitahu bahwa mereka tidak dapat mengubahnya oleh bos mereka pada saat itu, karena masalah waktu atau kompetensi, tetapi mungkin itu karena orang-orang yang picik.

Jika seorang penyelia berpikir j.random itu. Programmer tidak dapat melakukan refactoring, maka supervisor harus melakukannya.

Bagaimanapun ini terjadi, saya tahu kode ini ditulis oleh tim yang terbagi, dengan kemungkinan politik kekuasaan, dan mereka tidak memperbaiki desain subsistem yang tidak jelas.

Kisah nyata. Itu bisa saja terjadi padamu.

Adakah yang bisa memikirkan contoh di mana kode harus secara sah merujuk ke file dengan jalur absolut?

Menangkap pengecualian umum:

try {

 ...

} catch {
}

atau

try {

 ...

} catch (Exception ex) {
}

Wilayah terlalu sering digunakan

Biasanya, menggunakan terlalu banyak daerah menunjukkan kepada saya bahwa kelas Anda terlalu besar. Ini adalah bendera peringatan yang menandakan bahwa saya harus menyelidiki lebih dalam sedikit kode itu.

Konvensi penamaan kelas yang menunjukkan pemahaman yang buruk tentang abstraksi yang mereka coba ciptakan. Atau itu sama sekali tidak mendefinisikan abstraksi.

Sebuah contoh ekstrem muncul di benak saya di kelas VB yang pernah saya lihat yang diberi judul Datadan panjangnya 30.000+ ... di file pertama . Itu adalah sebagian kelas yang dibagi menjadi setidaknya setengah lusin file lainnya. Sebagian besar metode adalah pembungkus di sekitar procs yang disimpan dengan nama seperti FindXByYWithZ().

Bahkan dengan contoh-contoh yang kurang dramatis, saya yakin kita semua baru saja 'membuang' logika ke kelas yang kurang dipahami, memberinya gelar yang sepenuhnya generik, dan kemudian menyesalinya.

Fungsi yang menerapkan kembali fungsionalitas dasar bahasa. Misalnya, jika Anda pernah melihat metode "getStringLength ()" dalam JavaScript alih-alih panggilan ke properti ".length" dari string, Anda tahu Anda dalam masalah.

#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...

Tentu saja tanpa jenis dokumentasi dan bersarang sesekali #defines