2

I'm curious as to why this is. I ran into this scenario earlier today

using (SqlConnection oConn = new SqlConnection(ConnectionString))
{
    using (SqlCommand cmd = new SqlCommand("IC_Expense_InsertCycle", oConn))
    {
        cmd.CommandType = CommandType.StoredProcedure;
        cmd.Parameters.AddWithValue("@PortalId", portalId);
        cmd.Parameters.AddWithValue("@Description", description);
        cmd.Parameters.AddWithValue("@StartDate", start);
        cmd.Parameters.AddWithValue("@EndDate", end);

        try
        {
            oConn.Open();
            cmd.ExecuteNonQuery();
        }
        catch (SqlException ex)
        {
            throw ex;

        }
    }
}

//Get the new set of ExpenseCycles for binding
ExpenseCycle cycle = new ExpenseCycle(ConnectionString);
return cycle.GetExpenseCycles(portalId);

// ^^ this works just fine. The GetExpenseCycles call will basically set up the structure above with using SqlConnection and using SqlCommand

using (SqlConnection oConn = new SqlConnection(ConnectionString))
{
    using (SqlCommand cmd = new SqlCommand("IC_Expense_InsertCycle", oConn))
    {
        cmd.CommandType = CommandType.StoredProcedure;
        cmd.Parameters.AddWithValue("@PortalId", portalId);
        cmd.Parameters.AddWithValue("@Description", description);
        cmd.Parameters.AddWithValue("@StartDate", start);
        cmd.Parameters.AddWithValue("@EndDate", end);

        try
        {
            oConn.Open();
            cmd.ExecuteNonQuery();
        }
        catch (SqlException ex)
        {
            throw ex;

        }

        //Get the new set of ExpenseCycles for binding
        ExpenseCycle cycle = new ExpenseCycle(ConnectionString);
        return cycle.GetExpenseCycles(portalId);

        //This didn't work. The INSERT statement was successful, but it was bringing back old entries, and did not include the newest one that was just inserted
    }
}

The bottom code block was initially what I had, and the return count for my test environment was only 1, but there were 2 records in the database. It wasn't fetching that newly inserted record.

The basic code of GetExpenseCycles is the following:

using (SqlConnection oConn = new SqlConnection(ConnectionString))
{
    using (SqlCommand cmd = new SqlCommand("IC_Expense_GetExpenseCyclesByPortal",oConn))
    {
        oConn.Open();
        using (SqlDataReader sdr = cmd.ExecuteReader())
        {
            //Read List<expensecycle> here
        }
    }
}

Any ideas why? There were no exceptions thrown.

dgarbacz
  • 962
  • 1
  • 7
  • 17
  • Hrrrm, you think just having a generic throw would throw something else that's not `SqlException`? – dgarbacz Oct 12 '12 at 12:51
  • Unless you intend on doing something with the `SqlException` I wouldn't bother catching it just to re-throw it. – James Oct 12 '12 at 12:56
  • You have a return statement within the code, so it's obvious that the only the first insert is executed – Mohsen Afshin Oct 12 '12 at 12:58
  • 1
    a `catch(... ex)` which just does a `throw ex;` does nothing but harm... – Marc Gravell Oct 12 '12 at 13:00
  • @James: But it might be better to `throw` it in a catch than to omit the `try/catch` completely. It's easier to implement the missing error handling later(search for `throw;`) and it's obvious that this code block can raise an exception. – Tim Schmelter Oct 12 '12 at 13:01
  • @TimSchmelter, edited to show the structure of the .GetExpenseCycles method – dgarbacz Oct 12 '12 at 13:01
  • I thought it was bad practice to just do a generic `catch(Exception)... throw`? – dgarbacz Oct 12 '12 at 13:05
  • @dgarbacz: If the posted code of `GetExpenseCycles` is more or less complete, you've forgottten to set `cmd.CommandType = CommandType.StoredProcedure`. But that would cause an exception. – Tim Schmelter Oct 12 '12 at 13:05
  • 1
    @dgarbacz if you aren't going to do something useful, **don't catch it at all** - let it bubble automatically. All your throw does is destroy the stacktrace. That is the *only* thing it does; it would have continued rising anyway. – Marc Gravell Oct 12 '12 at 13:06
  • @TimSchmelter I would take that advice with a pinch of salt. If you *intend* to do something with it later i.e. error logging etc. fair enough otherwise don't catch it at all - it's just confusing & unnecessary. – James Oct 12 '12 at 13:07
  • @TimSchmelter It's less than complete, but the reader is executed, `while(sdr.Read())` builds a list and a `List` is returned – dgarbacz Oct 12 '12 at 13:07

2 Answers2

3

No exceptions thrown so no errors... I suspect the isolation-level on the connection

In the first scenario the connections don't overlap.

ExpenseCycle() use a connection string and I may safely assume it starts a new connection.

In the second example (problem case) the connections do overlap:

If the isolation-level is for instance read-committed and the "enclosing" connection hasn't yet stabilized its write (commit) the new connection don't pick up the changes, in this case the insert.

Possible solutions or things to try out: 1. Check the isolation-level on the connection 2. Pass the connection instead of the connectionstring to ExpenseCycle() (which is a better practice too imho)

lboshuizen
  • 2,746
  • 17
  • 20
1

You might have an ambient transaction in effect (if the code block is called within the scope of a transaction, new connections will join that transaction automatically. Using the TransactionScope class, you can get a handle of that transaction and commit it before the second call.

Also it looks like your second call is within the scope of the command's using block. Moving it outside of there might be enough to resolve your problem

using (SqlConnection oConn = new SqlConnection(ConnectionString)) 
{     
   using (SqlCommand cmd = new SqlCommand("IC_Expense_InsertCycle", oConn))     
   {
      cmd.CommandType = CommandType.StoredProcedure;
      cmd.Parameters.AddWithValue("@PortalId", portalId);
      cmd.Parameters.AddWithValue("@Description", description);
      cmd.Parameters.AddWithValue("@StartDate", start);
      cmd.Parameters.AddWithValue("@EndDate", end);          
      try
      {
         oConn.Open();
         cmd.ExecuteNonQuery();
      }
      catch (SqlException ex)
      {
         throw ex;
      }
   }//close the SqlCommand
   //Get the new set of ExpenseCycles for binding
   ExpenseCycle cycle = new ExpenseCycle(ConnectionString);
   return cycle.GetExpenseCycles(portalId);
   //This might fix your problem. 
} 

The other option is to move the second call outside the using of the first using block like so

bool insertSuccessful;
using (SqlConnection oConn = new SqlConnection(ConnectionString)) 
{     
   using (SqlCommand cmd = new SqlCommand("IC_Expense_InsertCycle", oConn))     
   {
      cmd.CommandType = CommandType.StoredProcedure;
      cmd.Parameters.AddWithValue("@PortalId", portalId);
      cmd.Parameters.AddWithValue("@Description", description);
      cmd.Parameters.AddWithValue("@StartDate", start);
      cmd.Parameters.AddWithValue("@EndDate", end);          
      try
      {
         oConn.Open();
         cmd.ExecuteNonQuery();
         insertSuccessful=true;
      }
      catch (SqlException ex)
      {
         insertSuccessful=false
         throw ex;
      }
   }//close the SqlCommand
}//close the connection
//Get the new set of ExpenseCycles for binding
if(insertSuccessful)
{
   ExpenseCycle cycle = new ExpenseCycle(ConnectionString);
   return cycle.GetExpenseCycles(portalId);
}

I think the first block should fix your problem. If not the second one definitely should.

Michael Brown
  • 9,041
  • 1
  • 28
  • 37