C290d3851c7837081d05b3491cbc331b

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

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 !

22e33503870d8e20493c4dd6b2f9767f

Rik Hemsley

April 2, 2008, April 02, 2008 13:29, permalink

2 ratings. Login to rate!

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();
            }
        }
    
C290d3851c7837081d05b3491cbc331b

robzyc

April 2, 2008, April 02, 2008 13:56, permalink

No rating. Login to rate!

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?

22e33503870d8e20493c4dd6b2f9767f

rikkus

April 2, 2008, April 02, 2008 15:00, permalink

No rating. Login to rate!

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.

C290d3851c7837081d05b3491cbc331b

robzyc

April 2, 2008, April 02, 2008 15:09, permalink

No rating. Login to rate!

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();
    }
}
22e33503870d8e20493c4dd6b2f9767f

rikkus

April 2, 2008, April 02, 2008 15:11, permalink

No rating. Login to rate!

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.

C290d3851c7837081d05b3491cbc331b

robzyc

April 2, 2008, April 02, 2008 15:25, permalink

No rating. Login to rate!

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?)

22e33503870d8e20493c4dd6b2f9767f

rikkus

April 2, 2008, April 02, 2008 16:05, permalink

No rating. Login to rate!

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!

C290d3851c7837081d05b3491cbc331b

robzyc

April 2, 2008, April 02, 2008 16:29, permalink

No rating. Login to rate!

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!

0b709acb624bc939041eb9483570d7a6

aifaz

July 21, 2008, July 21, 2008 07:43, permalink

No rating. Login to rate!

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??  
    }
C290d3851c7837081d05b3491cbc331b

robzyc

July 21, 2008, July 21, 2008 14:19, permalink

No rating. Login to rate!

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!

9e58b12a21a786c60c2fc956347e0c72

xesionprince

September 11, 2008, September 11, 2008 14:15, permalink

No rating. Login to rate!

Lol, you can't win em all Robzy ;)

9e58b12a21a786c60c2fc956347e0c72

Johnny

September 12, 2008, September 12, 2008 06:44, permalink

No rating. Login to rate!

LoL, if anyone can run before they can walk! I take my hat off to them! Clearly this is something you have'nt achieved ?? ;D

Your refactoring





Format Copy from initial code

or Cancel