Troubles with a C# function and SqlCommand that execute a stored procedure

Question

This is my first post here. I really like this place, I've found a lot of interesting stuff so far, so I decided to get aboard!

I'm experiencing something weird trying to use a recursive call in C#. This is what I'm doing.

//routine for downloading Phases
public static int DownloadPhases(string competitionId, string competitionCountry, string competitionPhase = "")
{
    DataTable dt = new DataTable();

    DateTime phaseStart = DateTime.Now, phaseEnd = DateTime.Now, outDate;

    XmlDocument xmlDoc = new XmlDocument();
    bool isDate = false, isNumeric = false;

    string url = "", phaseId = "", phaseCode = "", phaseName = "", phaseDesc = "";
    int phaseSort = 0, outNum = 0, result = 0;

    // load structure for inserting new Phases
    SqlUtilities.sqlLoadDt(ConfigurationManager.AppSettings["sqlSel_insCompetitionsPhase"], ref dt);

    DataColumn[] keyColumn2 = new DataColumn[3];
    keyColumn2[0] = dt.Columns["phase_ID"];
    keyColumn2[1] = dt.Columns["phase_Competition"];
    keyColumn2[2] = dt.Columns["phase_Country"];

    dt.PrimaryKey = keyColumn2;

    try
    {
        if (competitionPhase == "")
            url = buildUrl("15", "", "", competitionCountry, competitionId);
        else
            url = buildUrl("15", "", "", competitionCountry, competitionPhase);

        loadXml(xmlDoc, url);

        // set a nodelist with all the <category> tags for Phases
        XmlNodeList competitions = xmlDoc.GetElementsByTagName("Item");

        // for each node in the the nodelist, get all the infos about it
        foreach (XmlNode competition in competitions)
        {
            //phase
            phaseId = competition.Attributes.GetNamedItem("id").Value.ToString();
            phaseCode = competition.Attributes.GetNamedItem("code").Value.ToString();
            phaseName = competition.Attributes.GetNamedItem("name").Value.ToString();
            phaseDesc = competition.Attributes.GetNamedItem("description").Value.ToString();

            isDate = DateTime.TryParse(competition.Attributes.GetNamedItem("comp_start").Value.ToString(), out outDate);

            if (isDate == true) 
                phaseStart = outDate;

            isDate = DateTime.TryParse(competition.Attributes.GetNamedItem("comp_end").Value.ToString(), out outDate);

            if (isDate == true) 
               phaseEnd = outDate;

            isNumeric = int.TryParse(competition.Attributes.GetNamedItem("sort").Value.ToString(), out outNum);

            if (isNumeric == true) 
               phaseSort = outNum;

            // adding competition to datatable dt
            if (phaseId != "")
            {
               try
               {
                   dt.Rows.Add(phaseId, competitionId, competitionCountry, phaseCode, phaseStart, phaseEnd, phaseDesc, phaseName, phaseSort);
                   result += 1;

                   if (phaseDesc.Contains("NOT LEAF"))
                      <b>DlData.DownloadPhases(competitionId, competitionCountry, phaseId);</b>
               }
               catch (Exception ex)
               {
                    ex.Message.ToString();
               }
               finally
               {
                    phaseId = "";
               }
           }
        } //end foreach phase
     }
     catch (Exception ex)
     {
         ex.Message.ToString();
     }
     finally
     {
          //call stored procedure to insert Phases
          SqlUtilities.sqlExecuteSp("dbo.usp_insCompetitionsPhase", dt);
     }

     return result;
}

And this is the SqlExecuteSp function

public static void sqlExecuteSp(string cmdText, DataTable parDt)
{
  try
  {
    SqlConnection conn = new SqlConnection();

    SqlCommand comm = new SqlCommand();

    SqlParameter param = new SqlParameter();
    SqlTransaction trans;

    conn.ConnectionString = ConfigurationManager.ConnectionStrings["FTBLConnectionStringSuperUser"].ConnectionString;

    conn.Open();

    using (conn)
    {
      trans = conn.BeginTransaction();
      comm.CommandText = cmdText;
      comm.Connection = conn;
      comm.CommandType = CommandType.StoredProcedure;
      comm.Transaction = trans;

      param = comm.Parameters.AddWithValue("@dt", parDt);
      param.SqlDbType = SqlDbType.Structured;

      comm.ExecuteNonQuery();

      trans.Commit();
    }

    conn.Close();
  }
  catch (Exception ex)
  {
    ex.Message.ToString();
  }
}

When I try to call the SqlExecuteSp function recursively, it seems to execute it correctly, but I can't see the new rows inserted till when I delete the first ones.

So, If I run delete from tbl_competition_phases in SQL Server Management Studio, it only deletes the "old" rows, and from now on I can see the rows inserted with the second call.

I thought this happened due to an incomplete transaction so, as you can see, I added a transaction object, without results.

I know that recursive calls in c# are not the best thing to do. Anyway, in this case I'm quite sure that there won't be more than 2 calls, so I think it won't be a problem.

Any suggest?

Thank you very much!

Damiano

Answer 1

I think I've spotted what the error could be - in the sqlExecuteSp method you have your conn in a using block (which is declared incorrectly) - but are calling close outside of it after it has been disposed. This will be causing an exception. As I said in the comments - the fact you are swallowing exceptions is hiding this from you.

Try this for the sqlExecuteSp method

public static void sqlExecuteSp(string cmdText, DataTable parDt)
{
    using (SqlConnection conn = new SqlConnection())
    {
        using (SqlCommand comm = new SqlCommand())                    
        {
            conn.ConnectionString = ConfigurationManager.ConnectionStrings["FTBLConnectionStringSuperUser"].ConnectionString;
            conn.Open();

            using (SqlTransaction trans = conn.BeginTransaction())
            {
                comm.Connection = conn;
                comm.CommandType = CommandType.StoredProcedure;
                comm.Transaction = trans;

                SqlParameter param = comm.Parameters.AddWithValue("@dt", parDt);
                param.SqlDbType = SqlDbType.Structured;

                comm.ExecuteNonQuery();

                trans.Commit();
            }
        }

        conn.Close();
    }

}

this may not be the cause of the behaviour you are seeing - but would certainly cause an error / unexpected behaviour when the program runs.

A few tips

• enclose objects that can be disposed in using blocks to ensure they are disposed even if an error occurred (e.g. if your connection string was missing from the config file) and use the correct way of doing this as shown in my example
• Reduce the scope of your variables (only declare them when needed) - reduces the amount of time they have to exist and makes the code more readable (e.g. i moved the SqlParameter declaration down to only where its needed)
• Don't bother using try catch finally unless its part of a wider exception handling framework - or maybe only if you are wanting to log the exceptions and if you are logging rethrow so the error bubbles up. By using it where its not needed you are hiding errors which would have helped you to diagnose the problem. Sometimes its worth using try / finally alone to ensure a bit of code gets run - notice you don't have to have the catch block. this question has some useful reading on this