patterncsharpMinor
SQL service broker and threading
Viewed 0 times
sqlbrokerthreadingserviceand
Problem
Basically my code waits for database table
Are there any tips/suggestions that could be offered up? Are there any places which code is going to, for lack of better terms, explode and fail? I just want to make sure this is at least an acceptable way of completing the operations.
This code works perfectly, but does it make sense and can it be optimized?
Interface
Code
```
#region Declarations
private bool isPermitted()
{
try
{
SqlClientPermission clientPermission = new SqlClientPermission(PermissionState.Unrestricted);
clientPermission.Demand();
return true;
}
catch
{
return false;
lbxSystemAvailabilityReply.Items.Add("Failed permissions test.");
}
}
string db = Connect.GetDBString("DatabaseName");
static int RecordsProcessed = 0;
static decimal MaxRecordsRetained = 10;
const string SYSTEM_AVAILABILITY_QUERY = "SELECT [Columns] FROM [dbo].[System];";
const string SYSTEM_AVAILABILITY_REPLY_QUERY = "SELECT [Columns] FROM [dbo].[SystemReply];";
const string SYSTEM_AVAILABILITY_AWAIT = "SELECT [Columns] FROM [dbo].[System] WHERE [Columns] = 0;";
#endregion
public SystemAvailabilityLog()
{
InitializeComponent();
}
private void dbTransactionLog_Load(object sender, System.EventArgs e)
{
SqlDependency.Stop(db);
SqlDependency.Start(db);
Thread sysAvail = new Thread(SystemAvailabilityThread);
Thread sysAvailRep = new Thread(SystemAvailabilityReplyThread);
sysAvail.Start();
sysAvailRep.Start();
txtLastRestart.Text = DateTime.Now.ToSt
a to have an XML string inserted into it. The SQL query parses the XML string and stores it into separate columns (this has to occur this way because the insert is actually happening from elsewhere, I just rewrote if for testing). The service broker listens to database a and when it sees a new record inserted, it creates a new XML string to send out. I am not adept at threading and this is my very first time ever using the service broker in SQL.Are there any tips/suggestions that could be offered up? Are there any places which code is going to, for lack of better terms, explode and fail? I just want to make sure this is at least an acceptable way of completing the operations.
This code works perfectly, but does it make sense and can it be optimized?
Interface
Code
```
#region Declarations
private bool isPermitted()
{
try
{
SqlClientPermission clientPermission = new SqlClientPermission(PermissionState.Unrestricted);
clientPermission.Demand();
return true;
}
catch
{
return false;
lbxSystemAvailabilityReply.Items.Add("Failed permissions test.");
}
}
string db = Connect.GetDBString("DatabaseName");
static int RecordsProcessed = 0;
static decimal MaxRecordsRetained = 10;
const string SYSTEM_AVAILABILITY_QUERY = "SELECT [Columns] FROM [dbo].[System];";
const string SYSTEM_AVAILABILITY_REPLY_QUERY = "SELECT [Columns] FROM [dbo].[SystemReply];";
const string SYSTEM_AVAILABILITY_AWAIT = "SELECT [Columns] FROM [dbo].[System] WHERE [Columns] = 0;";
#endregion
public SystemAvailabilityLog()
{
InitializeComponent();
}
private void dbTransactionLog_Load(object sender, System.EventArgs e)
{
SqlDependency.Stop(db);
SqlDependency.Start(db);
Thread sysAvail = new Thread(SystemAvailabilityThread);
Thread sysAvailRep = new Thread(SystemAvailabilityReplyThread);
sysAvail.Start();
sysAvailRep.Start();
txtLastRestart.Text = DateTime.Now.ToSt
Solution
I see a couple of things in this piece of your code that I would like to address
-
There is a lot of nesting here, let's see what we can do about that.
Will become this
-
I am pretty sure that
Becomes this
-
Then there are a couple more Using statements nested deep in there that we can semi-combine without fancy Curly Braces
-
Empty Catch -> catch everything and hide it. This smells. I would get rid of it, but I don't know what you are doing with it either.
Here is what I came up with (Without the Try/Catch)
This Method also looks a little clunky
```
private void SystemAvailabilityTh
private void SystemAvailabilityReply()
{
try
{
if (isPermitted())
{
using (SqlConnection con = new SqlConnection(db))
{
using (SqlCommand cmd = CreateCommandWithDependency(SYSTEM_AVAILABILITY_AWAIT, con, SystemAvailabilityReply_OnChange))
{
con.Open();
using (SqlDataReader dr = cmd.ExecuteReader())
{
if (dr.HasRows)
{
while (dr.Read())
{
if (lbxSystemAvailabilityReply.Items.Count >= MaxRecordsRetained)
{
lbxSystemAvailabilityReply.Items.RemoveAt(0);
}
SystemAvailabilityReply sarPing = new SystemAvailabilityReply();
sarPing.PingID = dr["PingID"].ToString();
using (SqlConnection conInsert = new SqlConnection(db))
{
using (SqlDataReader drInsert = Connect.ExecuteReader("[db_InsertSystemAvailabilityReply]", conInsert,
new SqlParameter("@Param1", sarPing.PingID)))
{ }
}
string transactionDetail = string.Format("PingID: {0} PingDate: {1} PingTime: {2}",
sarPing.PingID, sarPing.PingDate, sarPing.PingTime);
lbxSystemAvailabilityReply.Items.Add(transactionDetail);
UpdateRecordCount();
}
}
}
}
}
}
}
catch (Exception ex)
{
}
}-
There is a lot of nesting here, let's see what we can do about that.
using (SqlConnection con = new SqlConnection(db))
{
using (SqlCommand cmd = CreateCommandWithDependency(SYSTEM_AVAILABILITY_AWAIT, con, SystemAvailabilityReply_OnChange))
{Will become this
using (SqlConnection con = new SqlConnection(db))
using (SqlCommand cmd = CreateCommandWithDependency(SYSTEM_AVAILABILITY_AWAIT, con, SystemAvailabilityReply_OnChange))
{-
I am pretty sure that
while (dr.Read()) won't read rows that don't exist in other words dr.Read() should be false if there are no rows to read, and that makes the check for rows redundant. So this:if (dr.HasRows)
{
while (dr.Read())
{Becomes this
while (dr.Read())
{-
Then there are a couple more Using statements nested deep in there that we can semi-combine without fancy Curly Braces
using (SqlConnection conInsert = new SqlConnection(db))
using (SqlDataReader drInsert = Connect.ExecuteReader("[db_InsertSystemAvailabilityReply]", conInsert,
new SqlParameter("@Param1", sarPing.PingID)))
{ }-
Empty Catch -> catch everything and hide it. This smells. I would get rid of it, but I don't know what you are doing with it either.
Here is what I came up with (Without the Try/Catch)
private void SystemAvailabilityReply()
{
if (isPermitted())
{
using (SqlConnection con = new SqlConnection(db))
using (SqlCommand cmd = CreateCommandWithDependency(SYSTEM_AVAILABILITY_AWAIT, con, SystemAvailabilityReply_OnChange))
{
con.Open();
using (SqlDataReader dr = cmd.ExecuteReader())
{
while (dr.Read())
{
if (lbxSystemAvailabilityReply.Items.Count >= MaxRecordsRetained)
{
lbxSystemAvailabilityReply.Items.RemoveAt(0);
}
SystemAvailabilityReply sarPing = new SystemAvailabilityReply();
sarPing.PingID = dr["PingID"].ToString();
using (SqlConnection conInsert = new SqlConnection(db))
using (SqlDataReader drInsert = Connect.ExecuteReader("[db_InsertSystemAvailabilityReply]", conInsert,
new SqlParameter("@Param1", sarPing.PingID)))
{ }
string transactionDetail = string.Format("PingID: {0} PingDate: {1} PingTime: {2}",
sarPing.PingID, sarPing.PingDate, sarPing.PingTime);
lbxSystemAvailabilityReply.Items.Add(transactionDetail);
UpdateRecordCount();
}
}
}
}
}This Method also looks a little clunky
```
private void SystemAvailabilityTh
Code Snippets
private void SystemAvailabilityReply()
{
try
{
if (isPermitted())
{
using (SqlConnection con = new SqlConnection(db))
{
using (SqlCommand cmd = CreateCommandWithDependency(SYSTEM_AVAILABILITY_AWAIT, con, SystemAvailabilityReply_OnChange))
{
con.Open();
using (SqlDataReader dr = cmd.ExecuteReader())
{
if (dr.HasRows)
{
while (dr.Read())
{
if (lbxSystemAvailabilityReply.Items.Count >= MaxRecordsRetained)
{
lbxSystemAvailabilityReply.Items.RemoveAt(0);
}
SystemAvailabilityReply sarPing = new SystemAvailabilityReply();
sarPing.PingID = dr["PingID"].ToString();
using (SqlConnection conInsert = new SqlConnection(db))
{
using (SqlDataReader drInsert = Connect.ExecuteReader("[db_InsertSystemAvailabilityReply]", conInsert,
new SqlParameter("@Param1", sarPing.PingID)))
{ }
}
string transactionDetail = string.Format("PingID: {0} PingDate: {1} PingTime: {2}",
sarPing.PingID, sarPing.PingDate, sarPing.PingTime);
lbxSystemAvailabilityReply.Items.Add(transactionDetail);
UpdateRecordCount();
}
}
}
}
}
}
}
catch (Exception ex)
{
}
}using (SqlConnection con = new SqlConnection(db))
{
using (SqlCommand cmd = CreateCommandWithDependency(SYSTEM_AVAILABILITY_AWAIT, con, SystemAvailabilityReply_OnChange))
{using (SqlConnection con = new SqlConnection(db))
using (SqlCommand cmd = CreateCommandWithDependency(SYSTEM_AVAILABILITY_AWAIT, con, SystemAvailabilityReply_OnChange))
{if (dr.HasRows)
{
while (dr.Read())
{while (dr.Read())
{Context
StackExchange Code Review Q#63267, answer score: 5
Revisions (0)
No revisions yet.