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

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.ExecuteNonQuery(cmd);
cmd.Parameters.Clear();

}
catch (Exception ex)
{

throw ex;

}
}

}

• Any reason why you're not using some ORM? – svick Apr 25 '13 at 16:48
• because I don't know how to use EF – NinjaDeveloper Apr 25 '13 at 17:42
• @a_Elnajjar I updated my answer with some important info about the parameter name i.e. using Database.BuildParameterName to create it. – RobH Apr 30 '13 at 8:06

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!

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.