Saya telah mewarisi beberapa kode buruk yang telah saya sertakan contoh singkat di bawah ini.
- Apakah ada nama untuk pola khusus ini?
Apa sajakah rekomendasi untuk refactoring ini?
// 0=Need to log in / present username and password // 2=Already logged in // 3=Inactive User found // 4=Valid User found-establish their session // 5=Valid User found with password change needed-establish their session // 6=Invalid User based on app login // 7=Invalid User based on network login // 8=User is from an non-approved remote address // 9=User account is locked // 10=Next failed login, the user account will be locked public int processLogin(HttpServletRequest request, HttpServletResponse response, int pwChangeDays, ServletContext ServContext) { }
Jawaban:
Kode buruk bukan hanya karena angka ajaib , tetapi karena menyatukan beberapa makna dalam kode kembali, menyembunyikan di dalamnya artinya kesalahan, peringatan, izin untuk membuat sesi atau kombinasi dari ketiganya, yang membuatnya menjadi input buruk untuk pengambilan keputusan.
Saya akan menyarankan refactoring berikut: mengembalikan enum dengan hasil yang mungkin (seperti yang disarankan dalam jawaban lain), tetapi menambahkan ke enum atribut yang menunjukkan apakah itu penolakan, pengabaian (saya akan membiarkan Anda melewati ini terakhir kali) atau jika tidak masalah (LULUS):
==> LoginResult.java <==
==> Severity.java <==
==> Test.java <==
Output untuk Test.java menunjukkan tingkat keparahan untuk setiap LoginResult:
Berdasarkan nilai enum dan tingkat keparahannya, Anda dapat memutuskan apakah pembuatan sesi dilanjutkan atau tidak.
EDIT:
Sebagai tanggapan terhadap komentar @ T.Sar, saya mengubah nilai yang mungkin dari keparahan ke PASS, WAIVER dan DENIAL alih-alih (OK, PERINGATAN dan KESALAHAN). Dengan cara itu jelas bahwa DENIAL (sebelumnya ERROR) bukan merupakan kesalahan per se dan tidak seharusnya diterjemahkan ke dalam melempar pengecualian. Penelepon memeriksa objek dan memutuskan apakah akan melempar pengecualian, tetapi DENIAL adalah status hasil yang valid yang dihasilkan dari panggilan
processLogin(...)
.sumber
Severity
Ini adalah contoh Obsesi Primitif - menggunakan tipe primitif untuk tugas "sederhana" yang akhirnya menjadi tidak begitu sederhana.
Ini mungkin sudah dimulai sebagai kode yang mengembalikan a
bool
untuk menunjukkan keberhasilan atau kegagalan, kemudian berubah menjadiint
ketika ada keadaan ketiga, dan akhirnya menjadi seluruh daftar kondisi kesalahan yang hampir tidak berdokumen.Refactoring khas untuk masalah ini adalah membuat kelas baru / struct / enum / objek / apa pun yang dapat lebih mewakili nilai yang dimaksud. Dalam hal ini, Anda dapat mempertimbangkan beralih ke
enum
yang berisi kondisi hasil, atau bahkan kelas yang bisa berisibool
untuk keberhasilan atau kegagalan, pesan kesalahan, informasi tambahan, dll.Untuk pola refactoring lebih lanjut yang mungkin berguna, lihat di Lembar Logika Bau Industri untuk Refactorings .
sumber
Saya menyebutnya "angka ajaib" - angka yang spesial dan tidak memiliki arti yang jelas dengan sendirinya.
Refactoring yang akan saya terapkan di sini adalah untuk merestrukturisasi tipe kembali ke enum, karena merangkum masalah domain dalam suatu jenis. Berurusan dengan kesalahan kompilasi yang jatuh dari itu harus dimungkinkan sedikit demi sedikit, karena java enum dapat dipesan dan diberi nomor. Bahkan jika tidak, seharusnya tidak sulit untuk berurusan dengan mereka secara langsung alih-alih kembali ke int.
sumber
if (processLogin(..) == 3)
Ini sedikit kode yang tidak menyenangkan. Antipattern dikenal sebagai "kode pengembalian ajaib". Anda dapat menemukan diskusi di sini .
Banyak dari nilai yang dikembalikan menunjukkan status kesalahan. Ada perdebatan yang valid tentang apakah akan menggunakan penanganan kesalahan untuk kontrol aliran, tetapi dalam kasus Anda, saya pikir ada 3 kasus: sukses (kode 4), sukses tetapi perlu mengubah kata sandi (kode 5), dan "tidak diizinkan". Jadi, jika Anda tidak peduli tentang menggunakan pengecualian untuk kontrol aliran, Anda bisa menggunakan pengecualian untuk menunjukkan status tersebut.
Pendekatan lain adalah untuk memperbaiki desain sehingga Anda mengembalikan objek "pengguna", dengan atribut "profil" dan "sesi" untuk login yang sukses, atribut "must_change_password" jika perlu, dan banyak atribut untuk menunjukkan mengapa log -dalam gagal jika itu alirannya.
sumber