تا جایی که دقت کردم (در بلاگهایی که منتشر میشوند) در آنسوی آبها، «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 یعنی تو خود شرح مفصل بخوان از این مجمل!