How do I improve my code using 3 tier architecture in ASP.NET C#

Question

0.00/5 (No votes)

See more:

C#

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using System.Web.UI;
using System.Web.UI.WebControls;
using System.Data;
using System.Data.SqlClient;
using BLLRHS;

namespace RHSWebApplication
{
    public partial class Home : System.Web.UI.Page
    {
        BLLClass obj = new BLLClass();
        protected void Page_Load(object sender, EventArgs e)
        {
            if (!IsPostBack)
            {
                BindGrid();
            }
        }
        
        public void BindGrid()
        {
            GridView1.DataSource = obj.SelectStudent();
            GridView1.DataBind();
        }
        
        public void Clear()
        {
            TextBox1.Text = ""; 
            RadioButtonList1.ClearSelection(); 
            TextBox2.Text = ""; 
            DropDownList1.Text = ""; 
            CheckBoxList1.Text = "";
        }

        protected void Button1_Click(object sender, EventArgs e)
        {
            obj.InsertData(TextBox1.Text, RadioButtonList1.SelectedItem.Text, TextBox2.Text, DropDownList1.SelectedItem.Text, CheckBoxList1.SelectedItem.Text);
            BindGrid();
            Clear();
        }

        protected void GridView1_RowEditing(object sender, GridViewEditEventArgs e)
        {
            GridView1.EditIndex = e.NewEditIndex;
            GridView1.DataSource = obj.SelectStudent();
            GridView1.DataBind();
        }

        protected void GridView1_RowUpdating(object sender, GridViewUpdateEventArgs e)
        {
            int Id = Convert.ToInt32(GridView1.DataKeys[e.RowIndex].Value);
            string Name = (GridView1.Rows[e.RowIndex].FindControl("TextBoxName") as TextBox).Text;
            string Gender = (GridView1.Rows[e.RowIndex].FindControl("RadioButtonListGender") as RadioButtonList).Text;
            string DOB = (GridView1.Rows[e.RowIndex].FindControl("TextBoxDOB") as TextBox).Text;
            string City = (GridView1.Rows[e.RowIndex].FindControl("DropDownListCity") as DropDownList).Text;
            string Qualification = (GridView1.Rows[e.RowIndex].FindControl("CheckBoxListQualification") as CheckBoxList).Text;
            obj.UpdateStudent(Id.ToString(), Name, Gender, DOB, City, Qualification);
            GridView1.EditIndex = -1;
            BindGrid();
        }

        protected void GridView1_RowDeleting(object sender, GridViewDeleteEventArgs e)
        {
            int Id = Convert.ToInt32(GridView1.DataKeys[e.RowIndex].Value);
            obj.DeleteStudent(Id.ToString());
            BindGrid();
        }

        protected void GridView1_RowCancelingEdit(object sender, GridViewCancelEditEventArgs e)
        {
            GridView1.EditIndex = -1;
            BindGrid();
        }

        protected void GridView1_RowDataBound(object sender, GridViewRowEventArgs e)
        {
            DataRowView dRowView = (DataRowView)e.Row.DataItem;
            if (e.Row.RowType == DataControlRowType.DataRow)
            {
                if ((e.Row.RowState & DataControlRowState.Edit) > 0)
                {
                    RadioButtonList rblGender = (RadioButtonList)e.Row.FindControl("RadioButtonListGender");
                    DropDownList ddlCity = (DropDownList)e.Row.FindControl("DropDownListCity");
                    CheckBoxList chkQua = (CheckBoxList)e.Row.FindControl("CheckBoxListQualification");
                    rblGender.SelectedValue = dRowView[2].ToString();
                    ddlCity.SelectedValue = dRowView[4].ToString();
                    chkQua.SelectedValue = dRowView[5].ToString();

                }
            }
        }
    }
}

Data Access Code as follows:

C#

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Data;
using System.Data.SqlClient;

namespace DALRHS
{
    public class DALClass
    {
        SqlConnection cn = new SqlConnection("Data Source=.;Initial Catalog=RHS;Integrated Security=True");
       
        public void InsertDetails(string Name, string Gender, string DOB,string City, string Qualification)
        {
            SqlCommand cmd = new SqlCommand("Sp_SaveData1", cn);
            cmd.CommandType = System.Data.CommandType.StoredProcedure;
            cmd.Parameters.AddWithValue("@Name", Name);
            cmd.Parameters.AddWithValue("@Gender", Gender);
            cmd.Parameters.AddWithValue("@DOB",DateTime.Parse (DOB));
            cmd.Parameters.AddWithValue("@City", City);
            cmd.Parameters.AddWithValue("@Qualification", Qualification);
            cn.Open();
            try
            {
                cmd.ExecuteNonQuery();
            }
            catch (Exception e)
            {
                Console.WriteLine(e.Message);
            }
            finally
            {
                cn.Close();
            }
        }
        public void UpdateDetail(string Id,string Name, string Gender, string DOB, string City, string Qualification)
        {
            SqlCommand cmd = new SqlCommand("spUpdate", cn);
            cmd.CommandType = System.Data.CommandType.StoredProcedure;
            cmd.Parameters.AddWithValue("@Id", Id);
            cmd.Parameters.AddWithValue("@Name", Name);
            cmd.Parameters.AddWithValue("@Gender", Gender);
            cmd.Parameters.AddWithValue("@DOB",DateTime.Parse(DOB));
            cmd.Parameters.AddWithValue("@City", City);
            cmd.Parameters.AddWithValue("@Qualification", Qualification);
            cn.Open();
            cmd.ExecuteNonQuery();
            cn.Close();
        }
        public object getDetail()
        {
            SqlDataAdapter da = new SqlDataAdapter("sp_GetAllStudent", cn);
            DataTable dt = new DataTable();
            da.Fill(dt);
            return dt;
        }
        public void deleteDetail(string Id)
        {
            SqlCommand cmd = new SqlCommand("Sp_Delete", cn);
            cmd.CommandType = System.Data.CommandType.StoredProcedure;
            cmd.Parameters.AddWithValue("@Id", Id);
            cn.Open();
            cmd.ExecuteNonQuery();
            cn.Close();
        }
    }
}

Bussiness Logic Layer as follows

C#

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using DALRHS;

namespace BLLRHS
{
    public class BLLClass
    {
        DALClass obj = new DALClass();

        public void InsertData(string Name, string Gender, string DOB, string City, string Qualification)
        {
            obj.InsertDetails(Name, Gender, DOB, City, Qualification);
        }
        public void UpdateStudent(string Id,string Name, string Gender, string DOB, string City, string Qualification)
        {
            obj.UpdateDetail(Id,Name, Gender, DOB, City, Qualification);
        }
        public object SelectStudent()
        {
           return  obj.getDetail();
        }
        public void DeleteStudent(string Id)
        {
            obj.deleteDetail(Id);
        }
    }
}

What I have tried:

Any Suggestion to improve my code?

Posted 30-Jul-21 19:51pm

raji vineeth 2021

Updated 2-Aug-21 0:07am

Richard Deeming

v3

Add a Solution

Comments

Asif 7969814 31-Jul-21 5:59am

Do not pass the value to function like this
public void InsertDetails(string Name, string Gender, string DOB, string City, string Qualification)

Use property

string Name {get; set;}
string Gender {get; set;}
string DOB {get; set;}
string City {get; set;}
string Qualification {get; set;}

public void InsertDetails()
{
SqlCommand cmd = new SqlCommand("Sp_SaveData1", cn);
cmd.CommandType = System.Data.CommandType.StoredProcedure;
cmd.Parameters.AddWithValue("@Name", Name);
cmd.Parameters.AddWithValue("@Gender", Gender);
cmd.Parameters.AddWithValue("@DOB",DateTime.Parse (DOB));
cmd.Parameters.AddWithValue("@City", City);
cmd.Parameters.AddWithValue("@Qualification", Qualification);
cn.Open();
try
{
cmd.ExecuteNonQuery();
}
catch (Exception e)
{
Console.WriteLine(e.Message);
}
finally
{
cn.Close();
}
}
when the table has lots of columns
this code style will reduce function complexity and typing efforts.

2 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Bohdan Stupak · Answer 1 · 2021-07-31T01:54:00

Here are a couple of things:
1. Instead of passing multiple primitive parameters in you methods you may introduce object (Student in your case)
2. From all your DAL methods only first closes connection inside finaly block. All of them should though since if something goes wrong you'll have opened connection.
3. Instead of returning object in your methods map output to Student class. There's no point in using statically typed language to pass object around
4. deleteDetail in your code has inconsistent naming. It's DeleteDetail
5. I'd rather avoid using string as a type of primary key in your db. I'd prefer int or Guid

Richard Deeming · Answer 2 · 2021-08-02T00:06:00

Some general suggestions:

Give your controls meaningful names; don't just accept the defaults provided by the Visual Studio designer.

Use model binding rather than retrieving control values manually:
Tutorial Series on Model Binding with ASP.NET Web Forms | ASP.NET Blog[^]
Dan Wahlin - Model Binding in ASP.NET 4.5 Web Forms[^]

Don't swallow exceptions. And don't use Console.WriteLine in an ASP.NET application.

Don't store a SqlConnection in a field. Instead, create it when it is needed, and wrap it in a using block to ensure it is disposed of as soon as possible.

Don't hard-code your connection string. Read it from the config file instead.

Wrap your SqlCommand objects in using blocks to ensure they're disposed of properly.

Give your stored procedures more meaningful names, and avoid using the sp_ prefix for them:
Is the sp_ prefix still a no-no? - SQLPerformance.com[^]

How do I improve my code using 3 tier architecture in ASP.NET C#

2 solutions

Solution 1

Solution 2

Add your solution here

Preview 0