1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44
void CopySourceTables(List<TableNameResult> tables, int id) { // Execute the "Copy" Command on the Server for Each Table. using (SqlConnection conn = tgtConnStr_.CreateConnection()) { conn.Open(); using (SqlCommand comm = new SqlCommand("sp_CopySourceTableToTargetTable", conn)) { // create params. SqlParameter source = new SqlParameter("@source", SqlDbType.VarChar, 255); source.Value = srcConnStr_.DatabaseName; comm.Parameters.Add(source); SqlParameter target = new SqlParameter("@target", SqlDbType.VarChar, 255); target.Value = tgtConnStr_.DatabaseName; comm.Parameters.Add(target); SqlParameter schema = new SqlParameter("@schema", SqlDbType.VarChar, 255); comm.Parameters.Add(schema); SqlParameter table = new SqlParameter("@table", SqlDbType.VarChar, 255); comm.Parameters.Add(table); SqlParameter tableID = new SqlParameter("@tableOID", SqlDbType.Int, 0); comm.Parameters.Add(tableID); SqlParameter snapshotID = new SqlParameter("@snapshotID", SqlDbType.Int, 0); comm.Parameters.Add(snapshotID); comm.CommandType = CommandType.StoredProcedure; comm.Prepare(); int tblCount = tables.Count; int currTbl = 0; foreach (TableNameResult tbl in tables) { schema.Value = tbl.User; table.Value = tbl.Table; tableID.Value = tbl.ID; snapshotID.Value = id; RaiseOpProgressEvent(new OpEventArgs("Copying Table Data: '" + table.Value + "'", tblCount, currTbl += 1)); comm.ExecuteNonQuery(); } } } }
Refactorings
No refactoring yet !
Rik Hemsley
April 2, 2008, April 02, 2008 13:29, permalink
If you're ever doing something slightly complex in a loop, it's likely it should be refactored into its own method.
You can use AddWithValue to clean up your parameter adding.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38
void CopySourceTables(List<TableNameResult> tables, int id)
{
// Execute the "Copy" Command on the Server for Each Table.
using (SqlConnection conn = tgtConnStr_.CreateConnection())
{
conn.Open();
int tblCount = tables.Count;
int currTbl = 0;
foreach (TableNameResult tbl in tables)
{
RaiseOpProgressEvent(new OpEventArgs("Copying Table Data: '" + tbl.Table + "'", tblCount, currTbl += 1));
CopySourceTable(conn, tbl, id);
}
}
}
private void CopySourceTable(SqlConnection conn, TableNameResult tbl, int id)
{
using (SqlCommand comm = new SqlCommand("sp_CopySourceTableToTargetTable", conn))
{
comm.CommandType = CommandType.StoredProcedure;
comm.Prepare();
comm.Parameters.AddWithValue("@source", strConnStr_.DatabaseName);
comm.Parameters.AddWithValue("@target", tgtConnStr_.DatabaseName);
comm.Parameters.AddWithValue("@schema", tbl.User);
comm.Parameters.AddWithValue("@table", tbl.Table);
comm.Parameters.AddWithValue("@tableOID", tbl.ID);
comm.Parameters.AddWithValue("@snapshotID", id);
comm.ExecuteNonQuery();
}
}
robzyc
April 2, 2008, April 02, 2008 13:56, permalink
Hi Rik,
Thanks for the response, makes complete sense, I am making changes as I type! It certainly does read a lot better :) I never knew about AddWithValue, so thanks for that!
One question though, does calling comm.Prepare for each table lose the benefit of the execution preparation? I would have thought it needed to be called outside the loop?
rikkus
April 2, 2008, April 02, 2008 15:00, permalink
I'm not sure what the Prepare call does, so I'm afraid you'll have to see if someone else knows what should be done with it.
robzyc
April 2, 2008, April 02, 2008 15:09, permalink
OK - no problem, I will look into that more.
Here's what I have so far. The stored procedures that are on the server differ slightly depending on backup/restore, I don't really want to go messing this them, so I added a little code to enable the Copy method to flip things around in restore mode...
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44
void CopyTables(List<TableNameResult> tables, int id, bool restore) { using (SqlConnection conn = tgtConnStr_.CreateConnection()) { // specify the sp to use and what db's are acting as source/target. // obviously if we are RESTORING, the data is coming from the specified // "Target" (i.e. snapshot DB), so we need to swap them over. string spName = restore ? "sp_RestoreSnapshotTable" : "sp_CopySourceTableToTargetTable"; string src = restore ? tgtConnStr_.DatabaseName : srcConnStr_.DatabaseName; string tgt = restore ? srcConnStr_.DatabaseName : tgtConnStr_.DatabaseName; conn.Open(); int tblCount = tables.Count; int currTbl = 0; foreach (TableNameResult tbl in tables) { RaiseOpProgressEvent( new OpEventArgs("Copying Table Data: '" + tbl.Table + "'", tblCount, currTbl += 1)); CopyTable(conn, src, tgt, tbl, id, spName); } } } void CopyTable(SqlConnection conn, string srcDB, string tgtDB, TableNameResult tbl, int snapshotID, string spName) { using (SqlCommand comm = new SqlCommand(spName, conn)) { comm.CommandType = CommandType.StoredProcedure; comm.Prepare(); comm.Parameters.AddWithValue("@source", srcDB); comm.Parameters.AddWithValue("@target", tgtDB); comm.Parameters.AddWithValue("@schema", tbl.User); comm.Parameters.AddWithValue("@table", tbl.Table); comm.Parameters.AddWithValue("@tableOID", tbl.ID); comm.Parameters.AddWithValue("@snapshotID", snapshotID); comm.ExecuteNonQuery(); } }
rikkus
April 2, 2008, April 02, 2008 15:11, permalink
See the comments by alazela here: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=1742482&SiteID=1 - it looks like Prepare isn't going to do anything when you're calling a stored procedure, so you might as well take it out.
robzyc
April 2, 2008, April 02, 2008 15:25, permalink
Ah ha! Great find Rik, thanks, I have learned stuff today! :)
comm.Prepare() deleted!
Thanks again for your help! How people gauge the ratings on this forum? Would I be right in assuming 5* is perfect?)
rikkus
April 2, 2008, April 02, 2008 16:05, permalink
To be honest I haven't looked at this site much as I just added the C# feed to my feed reader when I found it - and then forgot about it, so I'm not really sure about ratings.
Your code looks good to me now - the only thing left that's bothering me is the naming of various things ('restore' should be perhaps 'reverse', there are too many abbrvtns for my liking, etc.)
Cheers!
robzyc
April 2, 2008, April 02, 2008 16:29, permalink
Yeah same here, I added it ages ago then kind of forgot about it since not much has been happening on here. Recently started trying to dedicate some time to becoming more "agile", looking into TDD and obviously a huge part of this is the ability to refactor code effectively.
Hence me coming on here and just going for it, hopefully a few others will do the same and we can bring the site back to life. I don't want to join all the peeps in the ruby crowd, I think it looks awful! =(
Rated 4* since it needed a little tweak, nothing against you, it was more my fault, happy to change if people think otherwise, like I said, I'm a n00b! :)
Cheers Rik!
aifaz
July 21, 2008, July 21, 2008 07:43, permalink
how can i made class file, and how can i call it on default.aspx.cs
1 2 3 4 5 6 7 8 9 10
public Parameter parm(string str1, string str2) { comm.Parameters.Add(str1, SqlDbType.BigInt); comm.Parameters.Add(str2, SqlDbType.VarChar); con.Open(); comm.ExecuteNonQuery(); con.Close(); return?? }
robzyc
July 21, 2008, July 21, 2008 14:19, permalink
Great, my thread has been hijacked!
Purpose-of-website FAIL.
Posting-in-appropriate-thread FAIL
Explaining-problem-properly FAIL
Running-before-you-can-walk WIN!
xesionprince
September 11, 2008, September 11, 2008 14:15, permalink
Lol, you can't win em all Robzy ;)
Hi guys, following my previous post to see if people still lurk here, I have decided to throw myself to the wolves and submit my first code post!
With this code, I need use it twice, each one needing to use a different Stored Procedure name. So this is fine, I can bust it up to be a parameter. I am postive there is a better way to express this.
I am curious to see if you guys can improve the readability, since although it makes sense, it kinda looks messy.
The TableNameResult is a object that simply brings in the table info from another query.
Please be nice, but please be honest, I am here to learn from you good people :)
Many Thanks and Kind Regards,
Rob the n00b