Insert Selected item in listbox using Ado.net and stor proc

Question

I create this class method to Enter Selected Items Form CheckboxList to associated resource I use ADO.net & store Proc to insert data can someone help me to improve it and make to so clean instead of my Hacks BTW I use Microsoft Enterprise Lib

public static void Insert(List<int> listSkills, int rID)
 {
 var connection = ConfigurationManager.ConnectionStrings["SiteSqlServer"]; 
 Database objDB = new SqlDatabase(connection.ConnectionString);
 //int val = 0;
 using (DbCommand cmd = objDB.GetStoredProcCommand("Insert_Skills_Resources"))
 {
 foreach (var s in listSkills)
 {
 try
 {
 objDB.AddInParameter(cmd, "@SkillID", DbType.Int32, s);
 objDB.AddInParameter(cmd, "@ResourceID", DbType.Int32, rID);
 objDB.ExecuteNonQuery(cmd);
 cmd.Parameters.Clear();
 }
 catch (Exception ex)
 {
 throw ex;
 }
 }
 } 

• svick: Any reason why you're not using some ORM?
• NinjaDeveloper: because I don't know how to use EF
• RobH: @a_Elnajjar I updated my answer with some important info about the parameter name i.e. using Database.BuildParameterName to create it.

Answer 1

If you are using the Enterprise Library it is often better to set up a default connection in the configuration file:

<configuration>
 <configSections>
 <section name="dataConfiguration" 
 type="Microsoft.Practices.EnterpriseLibrary.Data.Configuration.DatabaseSettings,
 Microsoft.Practices.EnterpriseLibrary.Data, Version=5.0.414.0, 
 Culture=neutral, PublicKeyToken=null" 
 requirePermission="false"/>
 </configSections>
 <dataConfiguration defaultDatabase="SiteSqlServer" />
 <!-- Rest of your settings here, including your SiteSqlServer connection string -->
</configuration>

Now you don't need to look up the connection string - you can rely on the Enterprise library to get it for you:

public static void Insert(IEnumerble<int> skills, int resourceId)
{
 // No dependency on SQL anymore - the connection string is found
 // from the dataConfiguration section in your config file. If you change
 // to another (supported) provider, you don't need to change this code.
 Database db = DatabaseFactory.CreateDatabase();
 // Add in your schema to the stored proc name (I've assumed dbo).
 using (var cmd = db.GetStoredProcCommand("dbo.Insert_Skills_Resources"))
 {
 // This method populates the parameters collection of the DbCommand
 // by interrogating the database about the command.
 db.DiscoverParameters(cmd);
 // As the parameters are now known, we can just set the value of 
 // them directly, no need specify direction or type etc.
 cmd.Parameters["ResourceID"].Value = resourceId;
 foreach (var skill in skills)
 {
 cmd.Parameters["SkillID"].Value = skill;
 db.ExecuteNonQuery(cmd);
 // Don't need to clear the parameters now.
 }
 // <Pointless try catch removed.>
 }
} 

EDIT:

Sorry, it's been a year or more since I used the Enterprise Library I can't remember whether you need the @ at the start of the parameter name or not - it might be cmd.Parameters["@SkillID"].

EDIT 2:

I remembered why I couldn't remember if you need the @ or not (if that makes sense). You do need the @ BUT, you should use the Database.BuildParameterName method to create the parameter in the format the current provider expects - for sql server this is an @ symbol before the parameter name.

This is how you should do it:

cmd.Parameters[db.BuildParameterName("ResourceID")].Value = resourceId;

With this final correction the code is actually ignorant of the provider!

Answer 2

catch (Exception ex)
{
 throw ex;
}

This code doesn't make any sense. If you want to rethrow an exception, use just throw;, so that the stack trace is not reset. But if that's all you do in your catch block, then there is no reason to use try-catch, you should just remove it.