3

I have this code in a foreach loop that calls a sql function for each folder

foreach (string strCurrentFolder in strLocalSubFolderList)
{          
    SqlCommand sqlComm1 = new SqlCommand("dbo.fnChkXfer", _sqlConn);
    sqlComm1.CommandType = CommandType.StoredProcedure;
    sqlComm1.Parameters.Add("@FileXferred",SqlDbType.Bit).Direction = ParameterDirection.ReturnValue;
    sqlComm1.Parameters.AddWithValue("@UNCFolderPath", strCurrentFolder);
    sqlComm1.Parameters.AddWithValue("@FileType", "Type 1");
    ...if files not transferred, then transfer them

    SqlCommand sqlComm2 = new SqlCommand("dbo.fnChkXfer", _sqlConn);
    sqlComm2.CommandType = CommandType.StoredProcedure;
    sqlComm2.Parameters.Add("@FileXferred",SqlDbType.Bit).Direction = ParameterDirection.ReturnValue;
    sqlComm2.Parameters.AddWithValue("@UNCFolderPath", strCurrentFolder);
    sqlComm2.Parameters.AddWithValue("@FileType", "Type 2");
    ...if files not transferred, then transfer them

    SqlCommand sqlComm3 = new SqlCommand("dbo.fnChkXfer", _sqlConn);
    sqlComm3.CommandType = CommandType.StoredProcedure;
    sqlComm3.Parameters.Add("@FileXferred",SqlDbType.Bit).Direction = ParameterDirection.ReturnValue;
    sqlComm3.Parameters.AddWithValue("@UNCFolderPath", strCurrentFolder);
    sqlComm3.Parameters.AddWithValue("@FileType", "Type 3");
    ...if files not transferred, then transfer the
}                

It works fine, but there is a lot of duplicated code, which may be unnecessary.

Is it possible to create the SqlCommand object once outside of the loop and "reset" just the parameter values in the loop? How could I reuse the object? (Please be very specific).

Or should I continue to include this block of code in the loop, which executes the fn 3 times with different data for each iteration? It seems inefficient to keep recreating the SqlCommand object when it's almost exactly the same every time.

Maa421s
  • 159
  • 1
  • 12
  • You're calling fnChkXfer up to 3 times per item popped from your list and basically the difference is the value of @FileType? Seems like you could just have an inner for loop and break out of the loop if there's no reason to continue checking the files. – billinkc Aug 17 '15 at 16:52
  • That said, whatcha doing? Based on names, it appears you have a list of all the subfolders and based on this tsql function call, you're performing some OS level activity (assume copying files). It doesn't feel very ... SSIS-ey. I suspect we could use more of the out of the box components which would likely result in a more maintainable package with fewer defects if you could help us understand the problem you're attempting to solve – billinkc Aug 17 '15 at 16:55
  • Yes, that's correct. I'm calling the function 3 times and the difference is the file type. I have to determine if the files in the folders have been transferred for each type, then use WinSCP to transfer the files (from 3 locations on source to 2 diff locations on destination). I cannot use WinSCPs built in SyncFiles because the file structure is different on the 2 systems and I'm not rewriting working code to simplify this transfer (which is currently done manually). I am front ending the existing SSIS package with this transfer. The problem I'm solving is automating the SFTP transfer. – Maa421s Aug 18 '15 at 15:35
  • Perfect! Sometimes people reach for the familiar (.net) when they work with SSIS without exploring the native tasks which makes for a less good experience. It sounds like you have a very good reason for going this route though so apologies if I implied otherwise – billinkc Aug 18 '15 at 15:38
  • I cannot break out of checking anything. I have to check all 3 types for every folder. I would prefer to use more 'out of the box' features from WinSCP, but the folder structure prohibits this. I would prefer even more to be able to check for an actual file on the destination before transferring from the source, but there are no WinSCP commands to do this on both ends. – Maa421s Aug 18 '15 at 15:50
  • @billinkc Trust me, .NET is not familiar to me at all if that is not apparent. This is my first C# script with SSIS and it was the only way I could figure out how to do what I needed to. Thanks for your input. – Maa421s Aug 18 '15 at 15:52

2 Answers2

3

I changed your list to dictionary to add the Filetype. If you're going to reuse the same sqlcommand object, you need to execute it in every loop iteration. So I added that part. You may want to add a try, catch in there too.

Dictionary<string,string> strLocalSubFolderDict = new Dictionary<string, string>();

strLocalSubFolderDict.Add( "Type 1", "Directory 1");
strLocalSubFolderDict.Add( "Type 2", "Directory 2");
strLocalSubFolderDict.Add( "Type 3", "Directory 3");

using (SqlCommand sqlComm = new SqlCommand("dbo.fnChkXfer", _sqlConn))
{
     sqlComm.CommandType = CommandType.StoredProcedure;
     sqlComm.Parameters.Add("@FileXferred", SqlDbType.Bit).Direction = ParameterDirection.ReturnValue;
     sqlComm.Parameters.Add("@UNCFolderPath");
     sqlComm.Parameters.Add("@FileType");

     foreach (var val in strLocalSubFolderDict)
     {
         sqlComm.Parameters["@UNCFolderPath"].Value = val.Value;
         sqlComm.Parameters["@FileType"].Value = val.Key;
         sqlComm.ExecuteNonQuery();
         //    ...if files not transferred, then transfer them
      }
}

In this code the object is complete created outside the loop and the only changes in foreach are done to the values on the parameters of the object.

On a side, I'm not quite sure what you're doing with the FileXferred return parameter. Do you use it somewhere?

Update

Here's the code with where every directory has all the FileTypes applied to the directory.

        List<string> strLocalSubFolderList = new List<string>();
        List<string> typesList = new List<string>();

        typesList.Add("Type 1");
        typesList.Add("Type 2");
        typesList.Add("Type 3");

        using (SqlCommand sqlComm = new SqlCommand("dbo.fnChkXfer", _sqlConn))
        {
            sqlComm.CommandType = CommandType.StoredProcedure;
            sqlComm.Parameters.Add("@FileXferred", SqlDbType.Bit).Direction = ParameterDirection.ReturnValue;
            sqlComm.Parameters.Add("@UNCFolderPath");
            sqlComm.Parameters.Add("@FileType");

            foreach (var directval in strLocalSubFolderList)
            {
                foreach ( var typeval in typesList)
                {
                    sqlComm.Parameters["@UNCFolderPath"].Value = directval;
                    sqlComm.Parameters["@FileType"].Value = typeval;
                    sqlComm.ExecuteNonQuery();
                    //    ...if files not transferred, then transfer them
                }
            }
        }
dpimente
  • 487
  • 5
  • 9
  • @Billinkc, you're correct. It'll need to updated to reflect that. But the reusable single sqlcommand object idea is here. – dpimente Aug 17 '15 at 17:36
  • Thanks for the suggestion. I like the 2nd version and will implement that way. – Maa421s Aug 18 '15 at 15:41
1

You definitely can (and even should) create your SqlCommand outside the loop, and keep changing parameters as the loop progresses. In order to do that you need to store your parameters as you add them, and then set their values in the loop. You should also close the commands when you are done with them, preferably in some automated way. using statement is the most common choice for that.

using (var cmd = new SqlCommand("dbo.fnChkXfer", _sqlConn)) {
    cmd.CommandType = CommandType.StoredProcedure;
    cmd.Parameters.Add("@FileXferred",SqlDbType.Bit).Direction = ParameterDirection.ReturnValue;
    var p2 = cmd.Parameters.AddWithValue("@UNCFolderPath", DbType.Varchar, 32); // << Set the correct size
    var typeParam = cmd.Parameters.AddWithValue("@FileType", , DbType.Varchar, 32);
    foreach (string strCurrentFolder in strLocalSubFolderList) {
        p2.Value = strCurrentFolder;
        foreach (var typeVal in new[] {"Type 1", "Type 2", ...}) {
            typeParam.Value = typeVal;

            ... // Set values of the remaining parameters
            ... // Use your command as needed
        }
    }
}
// After this point all three commands will be closed automatically
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523