Dreaming of things

Code Analysis: Coding Standard (from who-cares code to code with care)

Pertama kali saya membuat Sistem Informasi yang komersil (yaitu Sistem Informasi Perikanan), dengan project management seadanya, dan proses yang "Ah gile" instead of Agile. Team Leader memberikan presentasi software yang baru kami selesaikan di depan Client. Hingga suatu titik muncul sebuah form dengan sebuah lable tertulis "Nama Ikan", dan di sebelah lable tersebut terdapat text box yang tertulis "Sapi". Ups!! Client bengong, Team Leader melirik pada saya dan menggugat dengan sebuah pertanyaan "Kok ada ikan namanya Sapi??"

JANGAN ditiru ya, hehe. Mungkin itu salah satu fase dan proses pembelajaran yang harus saya lalui untuk menjadi lebih professional. Sekali lagi, jangan ditiru!

Kecenderungan seorang mahasiswa (atau pelajar) dan fresh graduate (atau junior software engineer), memang kurang aware terhadap kualitas ketika membuat sebuah software. Oleh karena itu, proses dan manajemen lebih berperan dalam hal ini, seperti peran orang tua dalam membentuk karakter anaknya.

Berbicara tentang kualitas, Software yang berkualitas berawal dari code yang berkualitas. Saya akan coba memberikan contoh sederhana, sebuah code versi mahasiswa yang perlahan-lahan berubah menjadi code versi praktisi.

Masih ingat dengan program balik kata (reverse word)? Anda yang sudah berpengalaman dengan .NET mungkin langsung terlintas dengan fungsi Array.Reverse. Tentang Array.Reverse kita simpan buat nanti ya.. Sekarang, misal ada si Fulan seorang mahasiswa membuat fungsi reverse word seperti ini:

public static class WordUtility
{
  public static string rvwrd( string w )
  {
     string res = string.Empty;
     //for ( int i = w.Length; i > 0; i-- )
//{
//    string chr = w.Substring( i - 1, 1 );
//    x = x + chr;
//}

char c;
for ( int i = 1; i <= w.Length; i++ )
{
string chr = w.Substring( i - 1, 1 );
res = chr + res;
}

return res;
}
}

Compile OK, dan bisa berfungsi dengan benar, tidak ada masalah dengan compiler dan functionality-nya. Tapi code seperti ini adalah code yang buruk; jorok, susah dimengerti, dan tidak efektif. Terlalu banyak bad smell di sini, sehingga perlu dilakukan refactoring pada code tersebut.

Tulisan ini bakal agak overlap dengan serial refactoring-nya Ari. Bedanya, konteks serial refactoring-nya Ari benar-benar pada refactoring, sementara tulisan ini konteksnya adalah code analysis (refactoring hanya sebagai konsekuensi dari code analysis).

Sedikit catatan, code analysis (atau code review) selain secara otomatis (automated), bisa juga dilakukan dengan cara manual. Saya akan bahas sedikit tentang keduanya di artikel ini.

Kembali ke code di atas, kalau saya eksekusi code analysis di Visual Studio (detail dan teknis bagaimana cara melakukan automated code analysis, akan saya bahas pada artikel yang lain) terhadap code di atas, akan muncul empat warning sebagai berikut:

Warning 1 CA1709 : Microsoft.Naming : Correct the casing of 'rvwrd' in member name 'WordUtility.rvwrd(string)' by changing it to 'Rvwrd'.
Warning 2 CA1704 : Microsoft.Naming : Correct the spelling of 'rvwrd' in member name 'WordUtility.rvwrd(string)' or remove it entirely if it represents any sort of Hungarian notation.
Warning 3 CA1704 : Microsoft.Naming : In method 'WordUtility.rvwrd(string)', consider providing a more meaningful name than parameter name 'w'.
Warning 4 CA1804 : Microsoft.Performance : 'WordUtility.rvwrd(string)' declares a variable, 'c', of type 'char', which is never used or is only assigned to. Use this variable or remove it.

Mari kita bahas satu per satu..

Warning 1 CA1709 : Microsoft.Naming : Correct the casing of 'rvwrd' in member name 'WordUtility.rvwrd(string)' by changing it to 'Rvwrd'.
Penamaan method 'rvwrd' telah melanggar aturan penamaan (naming convention)-nya Microsoft. Sebuah method seharusnya berawalan huruf besar.

Warning 2 CA1704 : Microsoft.Naming : Correct the spelling of 'rvwrd' in member name 'WordUtility.rvwrd(string)' or remove it entirely if it represents any sort of Hungarian notation.
Masih pada penamaan method 'rvwrd', nama ini tidak terdefinisi dalam English grammar (terdapat spell checker pada FxCop untuk naming convention). Microsoft (FxCop) memberikan sugesti, jika terdapat Hungarian notation pada pengejaan 'rvwrd' sebaiknya dihilangkan. Saya tidak ingin membahas pro-kontra Hungarian notation di sini : ) Yang jelas, best practice dari Microsoft untuk naming convention .NET, tidak memperbolehkan adanya Hungarian notation, yang notabene Hungarian notation merupakan warisan dari Visual Basic classic.

So, untuk menghilangkan Warning 1 dan Warning 2, nama method 'rvwrd' perlu di-refactor menjadi 'ReverseWord'.

Warning 3 CA1704 : Microsoft.Naming : In method 'WordUtility.rvwrd(string)', consider providing a more meaningful name than parameter name 'w'.
Semua yang instan itu memang menyenangkan. Tapi tidak semua yang instan itu baik. Ini salah satunya. Di saat pembuatan, hanya dalam satu per sekian detik si fulan bisa mengetikkan parameter 'w'. Tapi dibutuhkan waktu yang jauh lebih banyak bagi orang lain untuk memahami parameter 'w' akan digunakan untuk apa di dalam method tersebut. Biasakan untuk memilih nama yang meaningful untuk penamaan class, property, method, parameter, variable, atau elemen apapun.

Kali ini, parameter 'w' perlu di-refactor menjadi 'word'.

Warning 4 CA1804 : Microsoft.Performance : 'WordUtility.rvwrd(string)' declares a variable, 'c', of type 'char', which is never used or is only assigned to. Use this variable or remove it.
Sering kita lihat code yang di dalamnya terdapat unused variable. Biasanya terjadi karena sebelumnya memang variable tersebut digunakan di dalam method, tapi ketika terjadi perubahan code, variable tersebut menjadi tidak terpakai. Atau, bisa juga karena ketika proses pembuatan method, variable tersebut dipersiapkan untuk keperluan mendatang "kalau-kalau" dibutuhkan. Ini penyakit flu dalam programming, harus segera disembuhkan. Jika memang variable tersebut tidak digunakan, hilangkan saja. Bayangkan jika variable tersebut memiliki type yang sangat kompleks, dan dalam satu program terdapat puluhan atau ratusan unused variable dengan type ini. Seberapa banyak kah memory yang akan terbuang? Apalagi jika software yang dibuat berskala besar seperti ERP, atau aplikasi CAD 3D yang rely on kapasitas memory. And this will surely end up with poor performance.

Setelah melakukan refactor, maka method di atas akan menjadi seperti ini:

public static class WordUtility
{
public static string ReverseWord( string word )
{
string res = string.Empty;
//for ( int i = w.Length; i > 0; i-- )
//{
//    string chr = w.Substring( i - 1, 1 );
//    x = x + chr;
//}

for ( int i = 1; i <= word.Length; i++ )
{
string chr = word.Substring( i - 1, 1 );
res = chr + res;
}

return res;
}
}

Sekarang.. Jalankan code analysis di Visual Studio sekali lagi dan hasilnya... kinclong! Tidak ada warning lagi : )

Merasa aneh?

Coding standard, termasuk naming convention, memang harus diimplementasikan pada keseluruhan code. Pada sebuah method, tidak hanya nama method dan parameter-nya saja yang harus memenuhi coding standard, tetapi implementasi method itu sendiri juga harus memenuhi coding standard. Kalau kita perhatikan method ReverseWord, implementasinya masih banyak terdapat pelanggaran/violation terhadap coding standard. Lalu kenapa code analysys pada Visual Studio (FxCop) tidak mendeteksi adanya violation di dalam method ReverseWord? Padahal terdapat naming convention violation pada local variable 'res' dan 'i'. Sayang sekali, untuk bisa melakukan code analysis dengan FxCop terhadap penamaan local variable, kita harus membuat custom rules dengan menggunakan FxCop SDK.

Alternatif solusinya adalah dengan melakukan manual code analysis/review.

Jika seorang reviewer menganalisa method ReverseWord ini dari aspek naming convention violation saja, maka ia akan meminta si Fulan untuk merubah nama local variable 'res', 'i', dan 'chr' karena tidak meaningful.

public static string ReverseWord( string word )
{
string reversedWord = string.Empty;
//for ( int i = w.Length; i > 0; i-- )
//{
//    string chr = w.Substring( i - 1, 1 );
//    x = x + chr;
//}

for ( int index = 1; index <= word.Length; index++ )
{
string character = word.Substring( index - 1, 1 );
reversedWord = character + reversedWord;
}

return reversedWord;
}

Tetapi seorang reviewer tidak hanya melakukan trivial code review seperti naming convention saja, semua coding standard perusahaan (atau coding standard project)--sebisa mungkin mendekati coding best practice--harus diterapkan dengan baik. Reviewer akan meminta perbaikan selanjutnya dengan menghilangkan comment yang tidak perlu dan memberikan sugesti supaya implementasi method ReverseWord menjadi lebih efektif.

Kalau Anda ingat, di awal tulisan ini, saya mengajak Anda untuk melupakan sejenak tentang Array.Reverse. Sekarang, saya masih tetap mengajak Anda untuk mengabaikan fungsi Array.Reverse, (untuk sementara) anggap saja tidak ada di .NET, hehe.

Sekali lagi, reviewer akan sangat kritis ketika melakukan code review. Looping dengan menggunakan foreach lebih efektif daripada for, seperti yang disebutkan oleh Bill Wagner pada bukunya yang berjudul Effective C#. Code yang efektif adalah diantara prioritas yang utama, seperti halnya penggunaan property instead of public variable, atau kapan saatnya menggunakan const dan kapan saatnya menggunakan static readonly, dan lainnya.

Lalu.. refactoring lagi...

public static class WordUtility
{
public static string ReverseWord( string word )
{
string reversedWord = string.Empty;
foreach( char character in word )
{
reversedWord = character + reversedWord;
}

return reversedWord;
}
}

Semakin rapih bukan?

Nah, tiba saatnya kita kembali mengingat fungsi Array.Reverse : ) Kesalahan pertama dan terbesar yang dilakukan si Fulan dalam kasus ini adalah; dia tidak mencoba untuk mencari tahu apakah sudah ada built-in function yang memadai untuk memecahkan masalah ReverseWord. Dan ternyata ada! Sang reviewer akan memberikan sugesti pada si Fulan untuk menggunakan built-in function Array.Reverse.

public static class WordUtility
{
public static string ReverseWord( string word )
{
char[] characters = word.ToCharArray();
Array.Reverse( characters );
return new string( characters );
}
}

Wow.. cantik yaa.. Seperti yang dikatakan oleh Ari, code itu layaknya bait-bait puisi; indah : )

Sebuah code analysis yang baik (manual ataupun automated), harus dapat memastikan bahwa code yang ada telah tercukupi dengan comment. Documentation comment (atau block comment) termasuk mandatory, apalagi jika code yang kita buat adalah API atau library yang akan digunakan oleh orang lain. Karena, walaupun kita sudah memakai naming convention dengan baik pada code kita, documentation akan sangat membantu orang lain untuk mengetahui secara detail cara penggunaan API atau library yang kita buat tadi. Saya menggunakan sebuah tool bernama GhostDoc untuk meng-construct documentation comment. Boleh dicoba, Free!

Tapi jangan salah, tidak hanya user (orang lain yang memakai code kita) saja yang memerlukan documentation, kita juga memerlukan documentation di dalam implementasi code kita untuk kebutuhan internal. Kalau kita kerja dalam tim, tentunya code yang kita buat pun harus bisa dimengerti oleh rekan-rekan tim kita, oleh karena itu commenting di dalam implementasi code yang kita buat (disebut inline comment) harus memadai.

So, class WordUtility dan method ReverseWord yang dipoles dengan comment akan menjadi seperti ini:

/// <summary>
/// A helper class for word processing.
/// </summary>
public static class WordUtility
{
/// <summary>
/// Reverses the word.
/// </summary>
/// <example>
/// <code>
/// string word = "Fajar Endra Nusa";
/// string reversedWord = WordUtility.ReverseWord( word );
/// // This will print 'asuN ardnE rajaF'.
/// Debug.Print( reversedWord );
/// </code>
/// </example>
/// <param name="word">The word.</param>
/// <returns>The reversed word.</returns>
public static string ReverseWord( string word )
{
// Get the array of characters.
char[] characters = word.ToCharArray();
// Reverse it prior to build the returned string.
Array.Reverse( characters );
return new string( characters );
}
}