Building session-variables from a SQL stored procedure

Question

Calling code and attempt at building session variables:

DataTable dtServerVars = clCommonFunctions.TryAutoLogin("europe\\MrTest");
Session["CablingUserID"]= dtServerVars.Rows[0]["CablingUserID"].ToString();
Session["CablingUseremail"] = dtServerVars.Rows[0]["CablingUseremail"].ToString();
Session["CablingLogin"] = dtServerVars.Rows[0]["CablingLogin"].ToString();
Session["CablingPassword"] = dtServerVars.Rows[0]["CablingPassword"].ToString();
Session["CablingPersonnel"] = dtServerVars.Rows[0]["CablingPersonnel"].ToString();
Session["CablingSurname"] = dtServerVars.Rows[0]["CablingSurname"].ToString();
Session["CablingFirstName"] = dtServerVars.Rows[0]["CablingFirstName"].ToString();
Session["CablingSuperUser"] = dtServerVars.Rows[0]["CablingSuperUser"].ToString();
Session["CablingDateAdded"] = dtServerVars.Rows[0]["CablingDateAdded"].ToString();
Session["CablingContact"] = dtServerVars.Rows[0]["CablingContact"].ToString();
Session["CablingApprovalAuthority"] = dtServerVars.Rows[0]["CablingApprovalAuthority"].ToString();
Session["CablingAdminUser"] = dtServerVars.Rows[0]["CablingAdminUser"].ToString();
Session["SharedInfoID"] = dtServerVars.Rows[0]["SharedInfoID"].ToString();
Session["SharedInfousername"] = dtServerVars.Rows[0]["SharedInfousername"].ToString();
Session["SharedInfopassword"] = dtServerVars.Rows[0]["SharedInfopassword"].ToString();
Session["SharedInfoname"] = dtServerVars.Rows[0]["SharedInfoname"].ToString();
Session["SharedInfoemail"] = dtServerVars.Rows[0]["SharedInfoemail"].ToString();
Session["SharedInfoICLlocation"] = dtServerVars.Rows[0]["SharedInfoID"].ToString();
Session["SharedInfoPhone"] = dtServerVars.Rows[0]["SharedInfoPhone"].ToString();
Session["SharedInfoSecLevel"] = dtServerVars.Rows[0]["SharedInfoSecLevel"].ToString();
Session["IMSUserID"] = dtServerVars.Rows[0]["IMSUserID"].ToString();
Session["IMSUserName"] = dtServerVars.Rows[0]["IMSUserName"].ToString();
Session["IMSIsAnonymous"] = dtServerVars.Rows[0]["IMSIsAnonymous"].ToString();
Session["IMSLastActivityDate"] = dtServerVars.Rows[0]["IMSLastActivityDate"].ToString();
Session["loggedin"] = "unknown";

Code being called:

public static DataTable TryAutoLogin(string strREMOTE_USER)
{
 SqlConnection siConnection = new SqlConnection();
 siConnection.ConnectionString = Databases.getDbConnectionString("csSharedInfo");
 siConnection.Open();
 SqlCommand seCmd = new SqlCommand("GetSignOnDetails", siConnection);
 seCmd.CommandType = CommandType.StoredProcedure;
 seCmd.Parameters.Add(new SqlParameter("@DomainAccount", SqlDbType.NVarChar, 300));
 seCmd.Parameters["@DomainAccount"].Value = strREMOTE_USER;
 seCmd.CommandType = CommandType.StoredProcedure;
 SqlDataAdapter sda = new SqlDataAdapter();
 seCmd.Connection = siConnection;
 sda.SelectCommand = seCmd;
 DataTable dtServerVars = new DataTable();
 sda.Fill(dtServerVars);
 siConnection.Close();
 if (dtServerVars != null)
 {
 if (dtServerVars.Rows.Count > 0)
 {
 return dtServerVars;
 }
}
 return null;
}

Answer 1

First your TryAutoLogin:

• A SqlConnection is IDisposable. Use it in combination with using.
• You have some duplicate lines of code, like seCmd.CommandType = CommandType.StoredProcedure;.
• dtServerVars can never be null.
• You do not have to open the connection, SqlDataAdapter will do that for you.
• Since you are using only ONE datarow, just return one datarow.

Resulting in:

 public static DataRow TryAutoLogin(string strREMOTE_USER)
 {
 using(SqlConnection siConnection = new SqlConnection(Databases.getDbConnectionString("csSharedInfo")))
 {
 SqlCommand seCmd = new SqlCommand("GetSignOnDetails", siConnection);
 seCmd.CommandType = CommandType.StoredProcedure;
 seCmd.Parameters.AddWithValue("@DomainAccount", strREMOTE_USER);
 SqlDataAdapter sda = new SqlDataAdapter(seCmd);
 DataTable dtServerVars = new DataTable();
 sda.Fill(dtServerVars);
 if (dtServerVars.Rows.Count > 0)
 return dtServerVars.Rows[0];
 return null;
 }
 }

Having done this, the next piece of code will also be a lot easier:

DataRow drServerVars = clCommonFunctions.TryAutoLogin("europe\\MrTest");
Session["CablingUserID"]= drServerVars["CablingUserID"].ToString();
Session["CablingUseremail"] = drServerVars["CablingUseremail"].ToString();
Session["CablingLogin"] = drServerVars["CablingLogin"].ToString();
...

• RobH: You could simplify if (dtServerVars.Rows.Count > 0) { return dtServerVars.Rows[0]; } return null; to just: return dtServerVars.AsEnumerable().FirstOrDefault(). You would need to add a using for System.Data.DataSetExtensions.
• Martin Mulder: RobH: True! But MoiD101 is still learning a lot of stuff, I was not willing in introducing him into new techniques like LINQ, but just showing him how he can improve his use on the components he is already using.
• MoiD101: Thank You Martin and thank you RobH for your contribution, as Martin commented though i have gota learn to walk first ;)
• MoiD101: Martin, just one question, the if statement below, is that some sort of short hand can i go read about it somewhere it seems very forein to me thats all...
• Martin Mulder: @RobH+MoiD101: RobH is correct. It is a personal chois with pro's and con's. It can lead to mistakes if your coding style (and how you implement it) is not consistent.

Answer 2

The part where you set session properties could be simplified a lot. If you want to get all columns from the DataTable into Session, then you can use the Columns collection. Something like:

DataTable dtServerVars = clCommonFunctions.TryAutoLogin("europe\\MrTest");
foreach (DataColumn column in dtServerVars.Columns)
{
 Session[column.ColumnName] = dtServerVars.Rows[0][column].ToString();
}

Though it seems some of your column names are different in Session. To do that, you could use a helper method:

private static string TranslateColumnName(string dataTableColumnName)
{
 switch (dataTableColumnName)
 {
 case "SharedInfoID":
 return "SharedInfoICLlocation";
 default:
 return dataTableColumnName;
 }
}

Session[TranslateColumnName(column.ColumnName)] = dtServerVars.Rows[0][column].ToString();

Also, if you know that all the columns are strings, I would use the Field() extension method instead of ToString(). That's because it handles DBNull properly and will throw an exception if the data in the column is actually a different type.

Session[TranslateColumnName(column.ColumnName)] = dtServerVars.Rows[0].Field<string>(column);

Answer 3

My only suggestion, in addition to existing answers, would be to use a domain object instead of a datatable and multiple session variables. That way you have a simple, strongly-typed way of accessing all those details at once without dealing with the database fields all over the place.

The idea would be to create a class for containing all the data:

public class SignOnDetails
{
 public int CablingUserID { get; set; }
 public string CablingUseremail { get; set; }
 public string CablingLogin { get; set; }
 public string CablingPassword { get; set; }
 //All other properties ommited for brevity, but it goes like the previous ones
}

Then, the TryAutoLogin method would return an instance of this class instead of a raw table:

public static SignOnDetails TryAutoLogin(string strREMOTE_USER)
{
 SqlConnection siConnection = new SqlConnection();
 siConnection.ConnectionString = Databases.getDbConnectionString("csSharedInfo");
 siConnection.Open();
 SqlCommand seCmd = new SqlCommand("GetSignOnDetails", siConnection);
 seCmd.CommandType = CommandType.StoredProcedure;
 seCmd.Parameters.Add(new SqlParameter("@DomainAccount", SqlDbType.NVarChar, 300));
 seCmd.Parameters["@DomainAccount"].Value = strREMOTE_USER;
 seCmd.CommandType = CommandType.StoredProcedure;
 SqlDataAdapter sda = new SqlDataAdapter();
 seCmd.Connection = siConnection;
 sda.SelectCommand = seCmd;
 DataTable dtServerVars = new DataTable();
 sda.Fill(dtServerVars);
 siConnection.Close();
 if (dtServerVars != null)
 {
 if (dtServerVars.Rows.Count > 0)
 {
 //Here we build the SignOnDetails instance from the datatable
 SignOnDetails details = new SignOnDetails();
 details.CablingUserID = dtServerVars.Rows[0]["CablingUserID"];
 details.CablingUseremail = dtServerVars.Rows[0]["CablingUseremail"];
 details.CablingLogin = dtServerVars.Rows[0]["CablingLogin"];
 details.CablingPassword = dtServerVars.Rows[0]["CablingPassword"];
 //All other properties ommited for brevity, but it goes like the previous ones
 return details ;
 }
 }
 return null;
}

Then, you save all the data at once in one go:

SignOnDetails details= clCommonFunctions.TryAutoLogin("europe\\MrTest");
Session["SignOnDetails"] = details;

And using it becomes strongly typed, requiring accesing a single session variable:

string login = ((SignOnDetails)Session["SignOnDetails"]).CablingLogin;

This has the advantages of being stringly typed, don't rely much on magic strings, and the database is nicely encapsulated in one method, while the rest of the program only knows about this class.