Make the method parameterized

Question

I have designed my application in a layered approach. I have a BusinessOP layer for each interface and a common data access layer. In my Data access layer I have Data Reader method like this.|

     public SqlDataReader executeQuerys(string query01)
    {
        SqlConnection con = null;
        SqlCommand com = null;
        try
        {
            con = new SqlConnection(DBConnect.makeConnection());
            con.Open();
            com = new SqlCommand(query01, con);
            return com.ExecuteReader(CommandBehavior.CloseConnection);
        }

        catch
        {
            com.Dispose();
            con.Close();
            throw;
        }

This is the code for my DBConnection layer.

     public static string makeConnection()
    {
        string con = ConfigurationManager.ConnectionStrings["MyDB.Properties.Settings.ConString"].ToString();
        return con;
    }

In my business layer I have methods like this each calling a specific stored procedure.

     public SqlDataReader getLGDivID(string divName)
    {
        string query = "EXEC getLGDivID'" + divName + "'";
        return new DataAccessLayer().executeQuerys(query);
    }

As my business operation layer is unsecure, I want to have it with parameterized query in here I'm using string concatenation to pass parameters. Can anyone hint me how to modify it?

Side note: You should always `.Dispose`, not only when you catch an exception. — Silvermind, Oct 11 '14 at 13:38

meda · Accepted Answer · 2014-10-11T14:11:41.767

You can change your function a little bit:

public SqlDataReader executeQuerys(string query01, string paramName, string value)
{
    SqlConnection con = null;
    SqlCommand com = null;
    try
    {
        con = new SqlConnection(DBConnect.makeConnection());
        con.Open();
        com = new SqlCommand(query01, con);
        com.Parameters.AddWithValue(paramName, value);
        com.Dispose();
        con.Close();
    }
    catch
    {
        com.Dispose();
        con.Close();
        throw;
    }
   return com.ExecuteReader(CommandBehavior.CloseConnection);
}

then to use it:

public SqlDataReader getLGDivID(string divName)
{
    string query = "EXEC getLGDivID @divName";
    return new DataAccessLayer().executeQuerys(query, "@divName", divName);
}

EDIT:

As @silvermind pointed out, you should dispose your connection properly. The way you have it now it will dispose connection only when you catch an exception.

This is bad, make use of IDisposable, for example:

public SqlDataReader executeQuerys(string query01, string paramName, string value)
{
    using (SqlConnection con = new SqlConnection(DBConnect.makeConnection()))
    {
        try
        {
            con.Open();
            com = new SqlCommand(query01, con);
            com.Parameters.AddWithValue(paramName, value);
        }
        catch(SqlException ex)
        {
            //Handle the exceptio
            //no need to dispose connection manually
            //using statement will take care of that
        }
    }
    return com.ExecuteReader(CommandBehavior.CloseConnection);
}

You could perhaps consider showing a proper disposal with a using block to improve the answer. :) — Silvermind, Oct 11 '14 at 13:40
@meda it gives me this error, Error DataAccessLayer.executeQuerys(string, string, string)': not all code paths return a value — ChathurawinD, Oct 11 '14 at 14:10
@chathwind it makes sense, check my edit, the latter function — meda, Oct 11 '14 at 14:12
@meda When I try to read the values in the data reader in the form code like this it SqlDataReader reader = new CandidateOP().getLGDivID(cmbLgDiv.Text); if (reader.HasRows) { while (reader.Read()) { templgDivID = (Int32)reader[0]; } reader.Close(); } It gives me the error Invalid attempt to call HasRows when reader is closed, but it worked with my previous code. What do you think the reason is..? — ChathurawinD, Oct 11 '14 at 14:28
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/62888/discussion-between-meda-and-chathwind). — meda, Oct 11 '14 at 14:34

Make the method parameterized

1 Answers1