اهميت code review

تا جايي كه دقت كردم (در بلاگ‌هايي كه منتشر مي‌شوند) در آنسوي آب‌ها، «code review» يك شغل محسوب مي‌شود. سازمان‌ها، شركت‌ها و امثال آن از مشاورين يا برنامه نويس‌هايي با مطالعه بيشتر دعوت مي‌كنند تا از كدهاي آن‌ها اشكال‌گيري كنند و بابت اينكار هم هزينه مي‌كنند.
اگر علاقمند باشيد قسمتي از يك پروژه سورس باز دريافت شده از همين دور و اطراف را با هم مرور كنيم:

//It's only for code review purpose!
protected void Button1_Click1(object sender, EventArgs e)
{        
        string  strcon;        
        string strUserURL;
        string strSQL;
        string strSQL1;
        strSQL = "SELECT UserLevel FROM listuser " + "WHERE Username='" + TextBox2.Text + "' " + "And Password='" + TextBox3.Text + "';";
        strSQL1 = "SELECT Pnumber FROM listuser " + "WHERE Username='" + TextBox2.Text + "' " + "And Password='" + TextBox3.Text + "';";
        strcon = @"Data Source=.\SQLEXPRESS;AttachDbFilename=|DataDirectory|\bimaran.mdf;Integrated Security=True;User Instance=True";
        SqlConnection myConnection = new SqlConnection(strcon);

        SqlCommand myCommand = new SqlCommand(strSQL, myConnection);
        SqlCommand myCommand1 = new SqlCommand(strSQL1, myConnection);
        myConnection.Open();

        strUserURL = (string)myCommand.ExecuteScalar();
        send = (string)myCommand1.ExecuteScalar();
        myCommand.Dispose();
        myCommand1.Dispose();
        myConnection.Close();


        if (strUserURL != null)
        {                      
            Label1.Text = "";

            url = "?Pn=" + code(send);
            FormsAuthentication.SetAuthCookie(TextBox2.Text, true);
            Response.Redirect("Page/" + strUserURL + url);
       }
        else
            Label3.Text = "چنین کاربری با این مشخصات ثبت نشده است.";
}

مروري بر اين كد يا «مشكلات اين كد»:
- كانكشن استرينگ داخل كدها تعريف شده. يعني اگر نياز به تغييري در آن بود بايد كدهاي برنامه تغيير كنند. آن هم نه فقط در اين تابع بلكه در كل برنامه.
- از پارامتر استفاده نشده. كد 100 درصد به تزريق اس كيوال آسيب پذير است.
- نحوه‌ي dispose شيء كانكشن غلط است. هيچ ضمانتي وجود ندارد كه كدهاي فوق سطر به سطر اجرا شود و خيلي زيبا به سطر بستن كانكشن استرينگ برسد. فقط كافي است اين ميان يك استثنايي صادر شود و تمام. به عبارتي اين سايت فقط با كمتر از 30 كاربر همزمان از كار مي‌افته. بعد نيايد بگيد من يك سرور دارم با 16 گيگ رم ولي باز كم مياره! همش برنامه كند ميشه. همش سايت بالا نمياد!
- همين تعريف كردن متغيرها در ابتداي تابع يعني اين برنامه نويس هنوز حال و هواي ANSI C را دارد!
- مهم نيست لايه بندي كنيد. ولي يك لايه در اين نوع پروژه‌ها الزامي است و آن هم DAL نام دارد. DAL يعني كثافت كاري نكنيد. يعني داخل هر تابع كُپه كُپه بر نداريد open و close بذاريد. بريد يك تابع يك گوشه‌اي درست كنيد كه اين عمليات را محصور كند.
- همين وجود Button1 و Label1 يعني تو خود شرح مفصل بخوان از اين مجمل!