Return to Revisions

@Malachi has pointed out a number of things I agree with. The level of nesting in that one procedure is called 'arrow-coding'.

The chained using blocks is common practice: using statement with multiple variables

There are two other things though that you can also do:

1. reverse the logic of the isPermitted() call to be:

    if (!isPermitted())
    {
        return;
    }
   

2. For nesting, and performance reasons, create just one insert connection for all the inserts you do. Bring the insert connection 'up' the stack to outside the loops:

    using (SqlConnection con = new SqlConnection(db))
    using (SqlConnection conInsert = new SqlConnection(db))
    using (SqlCommand cmd = CreateCommandWithDependency(SYSTEM_AVAILABILITY_QUERY, con, SystemAvailability_OnChange))
    {
        ....
    }
   

   This has the added benefit that the code only connects once for each insert, instead of once per insert. The connection is typically a slow process, so reducing the number of connect/disconnect instances is a good thing.

   The possible down-side here is if you typically insert nothing, but you will need to make the decision of whether the insert-nothing case outweighs the inserts-many-times.

One small style consistency problem you have is the empty-brace statement you have for the using on the executes. Sometimes you have:

 using (SqlDataReader dr = cmd.ExecuteReader()) { }

other times you have

 using (SqlDataReader drInsert = Connect.ExecuteReader("[db_InsertSystemAvailabilityReply]", conInsert,
         new SqlParameter("@Param1", sarPing.PingID)))
 { }