Apakah dianggap praktik yang buruk untuk memasukkan nomor bug dalam nama metode untuk penyelesaian sementara?

Rekan kerja saya yang adalah seorang pria senior memblokir saya pada ulasan kode karena dia ingin saya memberi nama metode 'PerformSqlClient216147Workaround' karena ini merupakan solusi untuk beberapa cacat ###. Sekarang, proposal nama metode saya adalah sesuatu seperti PerformRightExpressionCast yang cenderung menggambarkan apa metode yang sebenarnya dilakukan. Argumennya sejalan: "Ya metode ini hanya digunakan sebagai solusi untuk kasus ini, dan tidak di tempat lain."

Apakah memasukkan nomor bug di dalam nama metode untuk penyelesaian sementara dianggap praktik buruk?

Saya tidak akan menyebutkan metode seperti yang disarankan rekan kerja Anda. Nama metode harus menunjukkan apa yang dilakukan metode. Nama seperti PerformSqlClient216147Workaroundtidak menunjukkan apa fungsinya. Jika ada, gunakan komentar yang menjelaskan metode untuk menyebutkan bahwa ini merupakan solusi. Ini bisa terlihat seperti berikut:

/**
 * Cast given right-hand SQL expression.
 *
 * Note: This is a workaround for an SQL client defect (#216147).
 */
public void CastRightExpression(SqlExpression rightExpression)
{
    ...
}

Saya setuju dengan MainMa bahwa angka bug / cacat tidak boleh muncul dalam kode sumber itu sendiri melainkan dalam komentar kontrol sumber karena ini adalah meta-data, tetapi tidak buruk jika mereka muncul dalam komentar kode sumber. Nomor bug / cacat tidak boleh digunakan atas nama metode.

Tidak ada yang lebih permanen daripada perbaikan sementara yang berfungsi.

Apakah sarannya terlihat bagus dalam waktu 10 tahun? Biasanya merupakan praktik umum untuk mengomentari setiap perubahan dengan cacat yang diperbaiki. Baru-baru ini (seperti 3 dekade terakhir), komentar gaya ini diterima secara luas sebagai pengurangan pemeliharaan kode - dan itu hanya dengan komentar, bukan nama metode.

Apa yang dia usulkan adalah bukti kuat bahwa sistem QC dan QA Anda secara signifikan kurang. Pelacakan cacat dan perbaikan cacat termasuk dalam sistem pelacakan cacat, bukan kode sumber. Menelusuri perubahan kode sumber termasuk dalam sistem kontrol sumber. Referensi silang antara sistem ini memungkinkan penelusuran cacat ke kode sumber .....

Kode sumber ada untuk hari ini, bukan kemarin, dan bukan besok (seperti pada, Anda tidak mengetikkan ke sumber apa yang akan dilakukan tahun depan) ...

Jadi ini solusi sementara? Kemudian gunakan nama yang disarankan oleh peninjau, tetapi tandai metode ini sebagai usang, sehingga menggunakannya akan menghasilkan peringatan setiap kali seseorang menyusun kode.

Jika tidak, Anda mungkin selalu mengatakan bahwa itu 216147tidak masuk akal dalam kode, karena kode tersebut tidak ditautkan ke sistem pelacakan bug (ini lebih merupakan sistem pelacakan bug yang ditautkan dengan kontrol sumber). Kode sumber bukan tempat yang baik untuk referensi tiket bug dan versi, dan jika Anda benar-benar perlu meletakkan referensi itu di sana, lakukan di komentar.

Perhatikan bahwa bahkan dalam komentar, jumlah bug saja tidak terlalu berharga. Bayangkan komentar berikut:

public IEnumerable<Report> FindReportsByDateOnly(DateTime date)
{
    // The following method replaces FindReportByDate, because of the bug 8247 in the
    // reporting system.
    var dateOnly = new DateTime(date.Year, date.Month, date.Day);
    return this.FindReportByDate(dateOnly);
}

private IEnumerable<Report> FindReportsByDate(DateTime date)
{
    Contract.Requires(date.Hour == 0);
    Contract.Requires(date.Minute == 0);
    Contract.Requires(date.Second == 0);

    // TODO: Do the actual work.
}

Bayangkan bahwa kode itu ditulis sepuluh tahun yang lalu, bahwa Anda baru saja bergabung dengan proyek, dan ketika Anda bertanya di mana Anda dapat menemukan informasi tentang bug 8247, kolega Anda mengatakan bahwa ada daftar bug di situs web dari melaporkan perangkat lunak sistem, tetapi situs web itu direnovasi lima tahun yang lalu, dan daftar bug baru memiliki nomor yang berbeda.

Kesimpulan: Anda tidak tahu tentang apa bug ini.

Kode yang sama bisa ditulis dengan cara yang sedikit berbeda:

public IEnumerable<Report> FindReportsByDateOnly(DateTime date)
{
    // The reporting system we actually use is buggy when it comes to searching for a report
    // when the DateTime contains not only a date, but also a time.
    // For example, if looking for reports from `new DateTime(2011, 6, 9)` (June 9th, 2011)
    // gives three reports, searching for reports from `new DateTime(2011, 6, 9, 8, 32, 0)`
    // (June 9th, 2011, 8:32 AM) would always return an empty set (instead of isolating the
    // date part, or at least be kind and throw an exception).
    // See also: http://example.com/support/reporting-software/bug/8247
    var dateOnly = new DateTime(date.Year, date.Month, date.Day);
    return this.FindReportsByDate(dateOnly);
}

private IEnumerable<Report> FindReportsByDate(DateTime date)
{
    Contract.Requires(date.Hour == 0);
    Contract.Requires(date.Minute == 0);
    Contract.Requires(date.Second == 0);

    // TODO: Do the actual work.
}

Sekarang Anda mendapatkan pandangan yang jelas tentang masalah ini. Meskipun tampaknya tautan hiperteks di akhir komentar sudah mati lima tahun yang lalu, itu tidak masalah, karena Anda masih dapat memahami mengapa FindReportsByDatedigantikan oleh FindReportsByDateOnly.

Saya menemukan PerformSqlClient216147Workaroundlebih informatif kemudianPerformRightExpressionCast . Tidak ada keraguan sama sekali atas nama fungsi untuk apa fungsinya, mengapa ia melakukannya atau bagaimana cara mendapatkan informasi lebih lanjut tentangnya. Ini adalah fungsi eksplisit yang akan sangat mudah dicari dalam kode sumber.

Dengan sistem pelacakan bug yang secara unik mengidentifikasi nomor masalah, dan ketika Anda menarik bug dalam sistem itu menyediakan semua detail yang Anda butuhkan. Ini adalah hal yang sangat cerdas untuk dilakukan dalam kode sumber, dan akan menghemat waktu pengembang masa depan ketika mencoba memahami mengapa perubahan dilakukan.

Jika Anda melihat banyak nama fungsi ini jika kode sumber Anda, maka masalahnya bukan konvensi penamaan Anda. Masalahnya adalah Anda memiliki perangkat lunak kereta.

Ada nilai dalam saran rekan kerja Anda; itu memberikan keterlacakan dengan mengaitkan perubahan pada kode dengan alasan, didokumentasikan dalam database bug di bawah nomor tiket itu, mengapa perubahan itu dilakukan.

Namun itu juga menunjukkan bahwa satu-satunya alasan fungsi itu ada untuk mengatasi bug. Itu, jika masalahnya bukan masalah, Anda tidak ingin perangkat lunak melakukan hal itu. Agaknya Anda memang menginginkan perangkat lunak untuk melakukan hal tersebut, jadi nama fungsi harus menjelaskan apa yang dilakukannya dan mengapa domain masalah mengharuskan itu dilakukan; bukan mengapa database bug membutuhkannya. Solusinya harus terlihat seperti bagian dari aplikasi, bukan seperti bantuan band.

Saya pikir Anda dan dia mendapatkan semua ini di luar proporsi.

Saya setuju dengan argumen teknis Anda, tetapi pada akhirnya itu tidak akan membuat banyak perbedaan, terutama jika ini adalah perbaikan sementara yang dapat dihapus dari basis kode dalam beberapa hari / minggu / bulan.

Saya pikir Anda harus memasukkan ego Anda kembali ke dalam kotaknya, dan biarkan dia memiliki caranya sendiri. (Jika dia mendengarkan juga, aku akan mengatakan bahwa kalian perlu kompromi. Kedua ego kembali ke kotak mereka.)

Bagaimanapun, Anda dan dia memiliki hal-hal yang lebih baik untuk dilakukan.

Apakah memasukkan nomor bug di dalam nama metode untuk penyelesaian sementara dianggap praktik buruk?

Iya nih.

Idealnya, kelas Anda sebaiknya memetakan / mengimplementasikan konsep dalam aliran / keadaan aplikasi Anda. Nama-nama API dari kelas ini harus mencerminkan apa yang mereka lakukan terhadap keadaan kelas, sehingga kode klien dapat dengan mudah menggunakan kelas itu (yaitu tidak menentukan nama yang benar-benar tidak berarti apa-apa kecuali Anda membacanya secara khusus).

Jika bagian dari API publik dari kelas itu pada dasarnya mengatakan "lakukan operasi Y yang dijelaskan dalam dokumen / lokasi X" maka kemampuan orang lain untuk memahami API akan bergantung pada:

1. mereka mengetahui apa yang harus dicari dalam dokumentasi eksternal

2. mereka mengetahui ke mana harus mencari dokumentasi eksternal

3. mereka mengambil waktu dan usaha dan benar-benar mencari.

Kemudian lagi, nama metode Anda bahkan tidak menyebutkan di mana "lokasi X" adalah untuk cacat ini.

Itu hanya mengasumsikan (tanpa alasan realistis apapun) bahwa setiap orang yang memiliki akses ke kode, juga memiliki akses ke sistem pelacakan cacat dan bahwa sistem pelacakan masih ada selama kode stabil akan ada.

Paling tidak, jika Anda tahu cacat akan selalu dapat diakses di lokasi yang sama dan ini tidak akan berubah (seperti angka cacat Microsoft yang telah berada di URL yang sama selama 15 tahun terakhir), Anda harus memberikan tautan ke masalah dalam dokumentasi API.

Meski begitu, mungkin ada solusi untuk cacat lainnya, yang memengaruhi lebih dari API satu kelas. Apa yang akan anda lakukan selanjutnya?

Memiliki API dengan nomor cacat yang sama di beberapa kelas (data = controller.get227726FormattedData(); view.set227726FormattedData(data); tidak terlalu banyak memberi tahu Anda dan hanya membuat kode lebih kabur.

Anda dapat memutuskan bahwa semua cacat lainnya diselesaikan dengan menggunakan nama deskriptif operasi (data = controller.getEscapedAndFormattedData(); view.setEscapedAndFormattedData(data); ), kecuali dalam kasus 216147 cacat Anda (yang melanggar prinsip desain "paling tidak mengejutkan" - atau jika Anda ingin mengatakannya demikian, itu meningkatkan pengukuran "WTF / LOC" dari kode Anda).

TL; DR: Latihan buruk! Pergi ke kamarmu!

Tujuan utama seorang programmer harus kode kerja, dan, kejelasan ekspresi.

Memberi nama metode penyelesaian (.... Penanganan masalah). Tampaknya akan mencapai tujuan ini. Semoga pada tahap tertentu masalah yang mendasarinya akan diperbaiki dan metode penyelesaiannya dihapus.

Bagi saya, memberi nama metode setelah bug menunjukkan bahwa bug tersebut tidak terselesaikan atau root root tidak teridentifikasi. Dengan kata lain, itu menunjukkan masih ada yang tidak diketahui. Jika Anda mengatasi bug dalam sistem pihak ke-3, maka solusi Anda adalah solusi karena Anda tahu penyebabnya - mereka tidak bisa atau tidak akan memperbaikinya.

Jika bagian dari berinteraksi dengan SqlClient mengharuskan Anda untuk melakukan pemeran ekspresi yang benar, maka kode Anda harus membaca "performRightExpressionCast ()". Anda selalu dapat mengomentari kode untuk menjelaskan alasannya.

Saya telah menghabiskan 4 setengah tahun terakhir memelihara perangkat lunak. Salah satu hal yang membuat kode membingungkan untuk dipahami ketika melompat adalah kode yang ditulis dengan cara itu hanya karena sejarah. Dengan kata lain, itu bukan cara kerjanya jika ditulis hari ini. Saya tidak mengacu pada kualitas, tetapi ke sudut pandang.

Seorang rekan kerja saya pernah berkata, "Ketika memperbaiki cacat, buat kodenya seperti seharusnya." Cara saya mengartikannya adalah "Ubah kode menjadi seperti apa kelihatannya jika bug ini tidak pernah ada."

Konsekuensi:

1. Biasanya, lebih sedikit kode sebagai hasilnya.
2. Kode sederhana
3. Lebih sedikit referensi untuk bug dalam kode sumber
4. Lebih sedikit risiko regresi di masa mendatang (setelah perubahan kode diverifikasi sepenuhnya)
5. Lebih mudah untuk dianalisis karena pengembang tidak perlu membebani diri mereka dengan sejarah belajar yang tidak lagi relevan.

Kode sumber tidak perlu memberi tahu saya bagaimana statusnya saat ini. Kontrol versi dapat memberi tahu saya riwayatnya. Saya perlu kode sumber untuk hanya dalam keadaan diperlukan untuk bekerja. Yang mengatakan, komentar "// bug 12345" sesekali bukanlah ide yang buruk, tetapi dilecehkan.

Jadi ketika memutuskan apakah akan menyebutkan metode Anda 'PerformSqlClient216147Workaround', tanyakan pada diri sendiri pertanyaan-pertanyaan ini:

1. Jika saya tahu tentang bug 216147 sebelum menulis kode, bagaimana saya menanganinya?
2. Apa solusinya? Jika seseorang yang belum pernah melihat kode ini sebelumnya melihatnya, apakah mereka dapat mengikutinya? Apakah perlu memeriksa sistem pelacakan bug untuk mengetahui cara kerja kode ini?
3. Seberapa sementara kode ini? Dalam pengalaman saya, solusi sementara biasanya menjadi permanen, seperti yang ditunjukkan oleh @mattnz.