baseDALObject–Refactor

DAL in this case stands for Data Access Layer. Since this the application was built in a very data centric manner, I figure I will start my refactoring here.

In the DAL project, I found an object named baseDALObject. I am assuming this is a base class used by the other DAL objects. The fact that the objects and project is called DAL objects makes me cringe.

The original class file looks like:

1. using System;
2. using System.Data;
3. using System.Data.SqlClient;
4. //using Microsoft.ApplicationBlocks.Data;
5.  
6. namespace RMS_DALObjects
7. {
8.     /// <summary>
9.     /// Summary description for Class1.
10.     /// </summary>
11.     public class baseDALObject
12.     {
13.         #region "Variables"
14.  
15.         private string strConn = "connection_string";
16.         private SqlConnection sqlConn;
17.  
18.         #endregion
19.  
20.         #region "Constructors"
21.  
22.         public baseDALObject(){}
23.  
24.         #endregion
25.  
26.         #region "Methods"
27.  
28.         public DataSet getDataSet(string strSQL)
29.         {
30.             try
31.             {
32.                 return null;
33.                 //return SqlHelper.ExecuteDataset(strConn, CommandType.Text, strSQL);
34.             }
35.             catch (SqlException e)
36.             {
37.                 string msg = e.Message;
38.                 return null;    
39.             }
40.         }
41.  
42.         public SqlDataReader getDataReader(string strSQL)
43.         {
44.             try
45.             {
46.                 sqlConn = new SqlConnection(strConn);
47.                 sqlConn.Open();
48.  
49.                 SqlCommand sqlCmd = new SqlCommand(strSQL, sqlConn);
50.  
51.                 return sqlCmd.ExecuteReader();
52.             }
53.             catch (SqlException e)
54.             {
55.                 string msg = e.Message;
56.                 return null;    
57.             }
58.         }
59.  
60.         public void closeConnection()
61.         {
62.             try
63.             {
64.                 sqlConn.Close();
65.             }
66.             catch (SqlException e)
67.             {
68.                 string msg = e.Message;
69.             }
70.         }
71.  
72.  
73.  
74.  
75.         public SqlParameter[] getParameters(string storedProcedureName)
76.         {
77.             return null; //SqlHelperParameterCache.GetSpParameterSet(strConn, storedProcedureName);    
78.            }
79.  
80.         public void executeUpdate(string storedProcedureName, SqlParameter[] sqlParams)
81.         {
82.             try
83.             {
84.                 sqlParams[sqlParams.Length-3].Value = "RMS";
85.                 //SqlHelper.ExecuteNonQuery(strConn, CommandType.StoredProcedure, storedProcedureName, sqlParams);
86.             }
87.             catch (SqlException e)
88.             {    string msg = e.Message;    }
89.         }
90.  
91.         public void executeDelete(string storedProcedureName, SqlParameter[] sqlParams)
92.         {
93.             try
94.             {
95.                 //SqlHelper.ExecuteNonQuery(strConn, CommandType.StoredProcedure, storedProcedureName, sqlParams);
96.             }
97.             catch(SqlException e)
98.             {    string msg = e.Message;    }
99.         }
100.  
101.         #endregion
102.  
103.     }
104. }

I commented out the application block since I don’t have it installed and well I plan on removing it. I will give my past self a gold star that it only affected a single class.

But then I noticed I am catching exceptions and not throwing them… no idea what the hell I was thinking there.

So here are some steps I am taking to just reformat the code and basic refactoring.

1. Removing the regions

2. Getting rid of unused namespaces

3. Removing the try catch blocks (We can figure out exception handling later)

And I removed unused comments. These steps alone reduced the size of the file by half.

I am going to rename files as well to reflect a more common coding standard.

The resulting file is now:

1. using System.Data;
2. using System.Data.SqlClient;
3.  
4. namespace RMS_DALObjects
5. {
6.     public class BaseDALObject
7.     {
8.         SqlConnection sqlConn;
9.         string strConn = "connection_string";
10.  
11.         public DataSet GetDataSet(string strSQL)
12.         {
13.             return null;
14.             //return SqlHelper.ExecuteDataset(strConn, CommandType.Text, strSQL);
15.         }
16.  
17.         public SqlDataReader GetDataReader(string strSQL)
18.         {
19.             sqlConn = new SqlConnection(strConn);
20.             sqlConn.Open();
21.  
22.             SqlCommand sqlCmd = new SqlCommand(strSQL, sqlConn);
23.  
24.             return sqlCmd.ExecuteReader();
25.         }
26.  
27.         public void CloseConnection()
28.         {
29.             sqlConn.Close();
30.         }
31.  
32.         public SqlParameter[] GetParameters(string storedProcedureName)
33.         {
34.             return null; //SqlHelperParameterCache.GetSpParameterSet(strConn, storedProcedureName);    
35.         }
36.  
37.         public void ExecuteUpdate(string storedProcedureName, SqlParameter[] sqlParams)
38.         {
39.             sqlParams[sqlParams.Length - 3].Value = "RMS";
40.             //SqlHelper.ExecuteNonQuery(strConn, CommandType.StoredProcedure, storedProcedureName, sqlParams);
41.         }
42.  
43.         public void ExecuteDelete(string storedProcedureName, SqlParameter[] sqlParams)
44.         {
45.             //SqlHelper.ExecuteNonQuery(strConn, CommandType.StoredProcedure, storedProcedureName, sqlParams);
46.         }
47.     }
48. }

I am pretty sure we will eventually change this from using inheritance to using composition but for now, I will just leave it alone. I will change the code to use regular good ole ADO .net

You can’t change everything at once. But we can start to change one thing at a time. So although I am slightly confused as to why I need to keep the connection around and why some other object is using that connection and calling close, I will leave it as is.

But I will change the name of the sqlConn to _connection. I will also create a createConnection method the other methods can use.

It has also been awhile since I used plain ole vanilla ADO so forgive me if I do something wrong.

After a bit of tweaking the GetDataSet method now looks like

1.         public DataSet GetDataSet(string sql)
2.         {
3.             createConnection();
4.             var dataSet = new DataSet();
5.             new SqlDataAdapter(sql, _connection).Fill(dataSet);
6.             
7.             return dataSet;
8.         }

Refactoring the GetDataReader method results in

1.         public SqlDataReader GetDataReader(string sql)
2.         {
3.             createConnection();
4.             _connection.Open();
5.  
6.             return new SqlCommand(sql, _connection).ExecuteReader();
7.         }

Now the parameters methods I am going to have to think about. It’s rather late and that is a pretty good start.

Again check out the code at http://github.com/RookieOne/RMS_Refactor

In the future I should branch the code, but … I forgot.