HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

SQL service broker and threading

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
sqlbrokerthreadingserviceand

Problem

Basically my code waits for database table 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

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.