Click here to Skip to main content
15,885,546 members
Please Sign up or sign in to vote.
3.00/5 (3 votes)
See more:
Hi,

I am making a windows application, and I don't have any idea about splitting code like Business Layer, and Data Access Layer. Now, I have created UI as Windows Project, Business Layer as Class Library , and Data Access Layer as another Class Library.
I want to make sure that the below sample code I have created for Data Access Layer is right for the architecture. I am aiming to pass the values as Dictionary object to DAL, and DAL will call the stored procedure to perform the task requested by BL.
Is the below code technically correct ??

C#
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Data;
using System.Data.SqlClient;


namespace HRDAL
{
    public class procExec
    {
        SqlConnection con =null;
        public procExec(string conStr)
        {
            con = new SqlConnection(conStr);
            con.Open();

        }

        public KeyValuePair<int, string> ExecuteSP(string SPName, Dictionary<string, string> SPParameters)
        {
            if (con.State == ConnectionState.Open)
            {
                SqlCommand com = new SqlCommand();
                com.Connection = con;
                com.CommandType = CommandType.StoredProcedure;
                com.CommandText = SPName;
                foreach(KeyValuePair<string, string> Pm in SPParameters)
                    com.Parameters.Add(new SqlParameter(Pm.Key, Pm.Value));

                return new KeyValuePair<int, string>(1000, "Success");
            }
            else
                return new KeyValuePair<int, string>(1001, "No Connection to the Server");
        }

        public ~procExec()
        {
            con.Close();
        }
    }
}
Posted

1 solution

Ummm, well, OK.

My primary complaint is that you shouldn't hold the Connection Open the whole time; you should Open it and Close it in ExecuteSP.

Also, rather than a destructor, use a Dispose and have it call the Connection's Dispose.
Possibly put the Command in a using statement.
Personally I'd use IDbCommand com = con.CreateCommand() , but that's just me.

Most importantly, you don't execute the procedure.

Edit: I just realized that you may have threading issues if you're not careful -- consider locking the Connection.
 
Share this answer
 
v2
Comments
Yesudass Moses 21-Oct-14 19:42pm    
should I use interface on DAL ?
PIEBALDconsult 21-Oct-14 20:16pm    
You may, at least for the practice, but I don't think the app would really gain anything from it.
It should probably implement IDisposable.

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



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900