0

I am programmer in asp.net. I am using C#. I have written very lengthy code for query execution in each time. How to re-factor and organize the following code?

MySqlConnection connection = new MySqlConnection(connstring);
string query = "Select fo_region_Name from fo_region where fo_region_DeleteStatus=0";
MySqlCommand command = new MySqlCommand(query, connection);
MySqlDataReader reader;
connection.Open();
reader = command.ExecuteReader();
while (reader.Read())
{
    ddl_Country.Items.Add(UppercaseFirst(reader[0].ToString()));
}
connection.Close();

query = "Select Fo_Nationality_Name from fo_Nationality a, Fo_region b where a.Fo_Nationality_Type=1 and "
        + "LEFT(a.Fo_Nationality_Code,2)=LEFT(b.fo_region_Name,2)  and  a.Fo_Nationality_DeleteStatus=0 and "
        + "b.fo_region_DeleteStatus=0 Union Select Fo_Nationality_Name from fo_nationality where Fo_Nationality_DeleteStatus=0";
command = new MySqlCommand(query, connection);
connection.Open();
reader = command.ExecuteReader();
while (reader.Read())
{
    ddl_Nationality.Items.Add(UppercaseFirst(reader[0].ToString()));
}
connection.Close();

query = "select mcs_CreditCard_CardName from mcs_creditcard where mcs_CreditCard_DeleteStatus=0";
command = new MySqlCommand(query, connection);
connection.Open();
reader = command.ExecuteReader();
while (reader.Read())
{
    ddl_CreditCard.Items.Add(UppercaseFirst(reader[0].ToString()));
}
connection.Close();
Botz3000
  • 39,020
  • 8
  • 103
  • 127
Sagotharan
  • 2,586
  • 16
  • 73
  • 117

2 Answers2

2

Some thoughts:

  • Use multiline strings to format your SQL statements.
  • There is no need to close and reopen the connection betwween each command execution.
  • There is also no need to create new connection and command objects (in this case)
    • If you have parameters on the command objects, it is easier to create new command objects, rather than clearing out the old parameters
  • Use var statements to have the C# compiler automatically determine the variable type for you.
  • Wrap objects that need to be disposed, in a using block.

using (var connection = new MySqlConnection(connstring)) {
    connection.Open();

    using (var command = new MySqlCommand()) {
        MySqlDataReader reader;

        command.CommandText = @"
            SELECT fo_region_Name
            FROM fo_region
            WHERE fo_region_DeleteStatus=0
        ";
        using (reader = command.ExecuteReader()) {
            while (reader.Read()) {
                ddl_Country.Items.Add(UppercaseFirst(reader[0].ToString()));
            }
        }

        command.CommandText = @"
            SELECT Fo_Nationality_Name
            FROM fo_Nationality a,
                Fo_region b
            WHERE a.Fo_Nationality_Type = 1
                AND LEFT(a.Fo_Nationality_Code,2) = LEFT(b.fo_region_Name,2)
                AND b.fo_region_DeleteStatus=0

            UNION SELECT Fo_Nationality_Name 
            FROM fo_nationality
            WHERE Fo_Nationality_DeleteStatus=0
        ";
        using (reader = command.ExecuteReader()) {
            while (reader.Read()) {
                ddl_Nationality.Items.Add(UppercaseFirst(reader[0].ToString()));
            }
        }

        command.CommandText = @"
            SELECT mcs_CreditCard_CardName
            FROM mcs_creditcard
            WHERE mcs_CreditCard_DeleteStatus = 0
        ";
        using (reader = command.ExecuteReader()) {
            while (reader.Read()) {
                ddl_Nationality.Items.Add(UppercaseFirst(reader[0].ToString()));
            }
        }
    }
}

With LINQ (add a using System.Data.Common statement):

        using (reader = command.ExecuteReader()) {
            /*while (reader.Read()) {
                ddl_Country.Items.Add(UppercaseFirst(reader[0].ToString()));
            }*/
            ddl_Country.Items.AddRange((
                from DbDataRecord row in reader
                select new ListItem(
                    UppercaseFirst(reader.GetString(0))
                )
            ).ToArray());
        }
Zev Spitz
  • 13,950
  • 6
  • 64
  • 136
  • "There is no need to close and reopen the connection betwween each command execution." ... i would rather open and close it everytime and let the provider deal with pooling ... (http://stackoverflow.com/questions/4439409/open-close-sqlconnection-or-keep-open) –  Dec 18 '12 at 07:40
  • @AndreasNiedermair That post deals with static methods that each open and close the connection, where potentially each method could be called out of sequence. (The same would apply to multiple methods on a non-static class). In this case, the commands are executed sequentially. Within a single method, I think it's safe to keep the connection open for as long as needed. – Zev Spitz Dec 18 '12 at 07:51
  • 1
    @AndreasNiedermair Which is what `DbDataRecord row` does - it casts `row` to `DbDataRecord`. – Zev Spitz Dec 18 '12 at 07:52
  • 1
    one reason i'd never reuse command-objects: i'd have to think about all the parameters which are stored in that instance - should i drop 'em? or not? –  Dec 18 '12 at 07:54
  • one thought, that sums up your suggestions: i'd rather store individual steps in individual methods. no conflicts in sql-object-instance usages, better design, better readability,... and use `using`, and do not mess with internal connection-pooling, make it fail-safe with `try`/`catch`. –  Dec 18 '12 at 07:58
  • **The use of an explicitly typed range variable in a query expression is equivalent to calling the `Cast` method.** - [How to: Query an ArrayList with LINQ](http://msdn.microsoft.com/en-us/library/bb397937%28v=vs.100%29.aspx) – Zev Spitz Dec 18 '12 at 07:59
-2

Maybe use can use EnterpriseLibrary, to reduce amount of code that deals with database.

fedotoves
  • 757
  • 6
  • 7
  • 1
    maybe so, maybe not - who knows? c'mon, it's about facts, not about "ask the crystal ball" ... either the op elaborates or you should enhance your answer! –  Dec 18 '12 at 07:45