Namespaces:
Remove the unused namespaces and separate the Asp.Net framework namespaces with third party or other frame work spaces as below.
This makes the identification of namespaces at first look itself.
Always use Response. Redirect with false as its second parameter value
Response.Redirect("Response.aspx?ErrorID=1000",false);
Conditions Format
if (!(Session["FullName"] == null))
Use the below format for better readability
if (Session["FullName"] != null)
String Conversion not required on control’s text values
if (!string.IsNullOrEmpty(txtComment.Text.ToString().Trim()))
{
}
txtComment.Text itslef will return the string value and again don’t need to convert that value using ToString() method this looks odd, pleease remove this.
Use Page.IsValid property under submit/resubmit event handlers
if (Page.IsValid)
{
//Show some message to the user saying the validations are not met on page/ show validation summary control
return;
}
This helps avoiding the unexpected behavior of pages when validations are not firing because of JavaScript errors.
Add sub functions for main functions
If possible try to reduce the no’ of lines of code for each function by diving the functionality into sub functions (common logic).
Exception rising
catch (Exception ex)
{
//objCommonMethods.GenericExceptionMethod(ex);
spnErrorMsg.Visible = true;
objCommonMethods.DisplayMessage(this.spnErrorMsg, "Error", "Error Occured While Page Loading.");
}
Here we are not sowing the actual exception; this will not help in trouble shooting the issues in production. So please add the inner exception message to the display message.
catch (Exception ex)
{
//objCommonMethods.GenericExceptionMethod(ex);
spnErrorMsg.Visible = true;
objCommonMethods.DisplayMessage(this.spnErrorMsg, "Error", "Error Occured While Page Loading." + ”
” ex.Message +”
” + ex.StackTrace);
}
ex.StackTrace will give us the line number of code which is throwing the exception.
When throwing exception use the below format:
catch (Exception ex)
{
// throw ex;
throw new Exception("Exception in GetApprovers method", ex);
}
Use StringBuilder over strings when string concatenation is required more than 2 levels:
Use length condition before using string remove methods ***
Remove web related and unused namespaces in class files.
For string comparison use string.Compare()
With comparison method:
if (string.Compare(CompairFile.Trim(),InFile.Trim(),false) == 0)
{
file.Delete();
break;
}
Remove the commented code in all the files
Use count condition check before using a list enumeration in foreach loop
Ex:
foreach (DataRow drRole in dsApprovers.Tables[0].Rows)
{}
If the dsApprovers.Tables[0] doesn’t have any rows, it throws an exception.
Please check the for the same in other places.
Use DataTables over Datasets
For functions we are using Dataset in many places, but I think it will make sense when we use Data Table directly. Usually we use Dataset when we fill multi table’s data with relation. In our case we are nowhere storing multiple tables into Dataset. So using Data Tables is suggestible.
Move all the connection strings and constant values those are coming form web.config file can be moved to CommonConstantVariables class and can be made them static properties
Make Page level viewstate and session false for static pages like Response.aspx, confirmation.aspx and Unauthorized.aspx. This helps the pages load faster.
Example:
Remove the unused namespaces and separate the Asp.Net framework namespaces with third party or other frame work spaces as below.
This makes the identification of namespaces at first look itself.
Always use Response. Redirect with false as its second parameter value
Response.Redirect("Response.aspx?ErrorID=1000",false);
Conditions Format
if (!(Session["FullName"] == null))
Use the below format for better readability
if (Session["FullName"] != null)
String Conversion not required on control’s text values
if (!string.IsNullOrEmpty(txtComment.Text.ToString().Trim()))
{
}
txtComment.Text itslef will return the string value and again don’t need to convert that value using ToString() method this looks odd, pleease remove this.
Use Page.IsValid property under submit/resubmit event handlers
if (Page.IsValid)
{
//Show some message to the user saying the validations are not met on page/ show validation summary control
return;
}
This helps avoiding the unexpected behavior of pages when validations are not firing because of JavaScript errors.
Add sub functions for main functions
If possible try to reduce the no’ of lines of code for each function by diving the functionality into sub functions (common logic).
Exception rising
catch (Exception ex)
{
//objCommonMethods.GenericExceptionMethod(ex);
spnErrorMsg.Visible = true;
objCommonMethods.DisplayMessage(this.spnErrorMsg, "Error", "Error Occured While Page Loading.");
}
Here we are not sowing the actual exception; this will not help in trouble shooting the issues in production. So please add the inner exception message to the display message.
catch (Exception ex)
{
//objCommonMethods.GenericExceptionMethod(ex);
spnErrorMsg.Visible = true;
objCommonMethods.DisplayMessage(this.spnErrorMsg, "Error", "Error Occured While Page Loading." + ”
” ex.Message +”
” + ex.StackTrace);
}
ex.StackTrace will give us the line number of code which is throwing the exception.
When throwing exception use the below format:
catch (Exception ex)
{
// throw ex;
throw new Exception("Exception in GetApprovers method", ex);
}
Use StringBuilder over strings when string concatenation is required more than 2 levels:
Use length condition before using string remove methods ***
Remove web related and unused namespaces in class files.
For string comparison use string.Compare()
With comparison method:
if (string.Compare(CompairFile.Trim(),InFile.Trim(),false) == 0)
{
file.Delete();
break;
}
Remove the commented code in all the files
Use count condition check before using a list enumeration in foreach loop
Ex:
foreach (DataRow drRole in dsApprovers.Tables[0].Rows)
{}
If the dsApprovers.Tables[0] doesn’t have any rows, it throws an exception.
Please check the for the same in other places.
Use DataTables over Datasets
For functions we are using Dataset in many places, but I think it will make sense when we use Data Table directly. Usually we use Dataset when we fill multi table’s data with relation. In our case we are nowhere storing multiple tables into Dataset. So using Data Tables is suggestible.
Move all the connection strings and constant values those are coming form web.config file can be moved to CommonConstantVariables class and can be made them static properties
Make Page level viewstate and session false for static pages like Response.aspx, confirmation.aspx and Unauthorized.aspx. This helps the pages load faster.
Example:
