2

I am programming in Windows Forms and MySQL.

If I declare this in the program, I can use the connection and command objects in the whole .cs page:

 MySqlConnection connection = null;
 MySqlCommand command = null;
 MySqlDataReader Reader;

But if I write the code below, connection and command objects are declared twice:

 private void cmbo_class_SelectedValueChanged(object sender, EventArgs e)
        {
            string clas = cmbo_class.SelectedItem.ToString();            

            try
            {
                MySqlConnection connection = new MySqlConnection(hp.myConnStr);
                MySqlCommand command = connection.CreateCommand();
                MySqlDataReader Reader;
                command.CommandText = "select id,code from reg_class_master where name ='" + clas + "' and Delete_Status=0";
                connection.Open();
                Reader = command.ExecuteReader();
                while (Reader.Read())
                {
                    class_id = Convert.ToInt32(Reader[0].ToString());
                    class_code = Reader[1].ToString();
                }
                connection.Close();
            }
            catch(Exception ex)
            {
                MessageBox.Show("Error in ","Information",MessageBoxButtons.OK,MessageBoxIcon.Information);
            }

            cmbo_division.Items.Clear();

            try
            {
                MySqlConnection connection = new MySqlConnection(hp.myConnStr);
                MySqlCommand command = connection.CreateCommand();
                MySqlDataReader Reader;
                command.CommandText = "select name from reg_division_master where class_id = " + class_id + " and Delete_Status=0 order by display_index";
                connection.Open();
                Reader = command.ExecuteReader();
                while (Reader.Read())
                {
                    cmbo_division.Items.Add(Reader[0].ToString());
                }
                connection.Close();
            }
            catch
            {
            }

I think one object declaration is the best way. Do more object declarations affect the program?

0

4 Answers 4

3

Here are some rules that are almost always correct:

You should never reuse a variable for two different purposes.

The case when you need to, is a very specific one, and when you see it, you will know it (the flyweight pattern).

You should keep your methods very short.

The code you've shown can be logically contained in 3-5 methods at least.

You should not have duplicate code. (DRY = Don't Repeat Yourself)

Any similarly looking lines of code are most likely same functionality repeated in several places. Extract this in a single method.

2
  • Where can i read this type of rules?.
    – Sagotharan
    Commented Dec 29, 2011 at 10:48
  • 2
    Clean Code is one of the good books on the subject. Commented Dec 29, 2011 at 12:06
3

For this concrete case, I think you have correctly identified the two almost identical declarations and initializations of the connection, command and reader as a problem. This maybe a gross oversimplification, but I live by wherever there is duplication, there is a design problem.

I would therefore move the delcaration of the connection etc. to the top of the method, do the initialization there once and then only change the command text at the two points of use. If the connection is used repeatedly throughout your application, you might even consider creating it only once at application start-up.

Generally speaking, there are (at least) two things to balance:

  • Limited variable scope: On the one hand, you want to delcare variables as close to where they will be used as possible, so you will never have to wonder where a variable came from, how it has been initialized etc. and you won't be tempted to re-use a variable for something else.
  • Resource consumption and performance: Constantly opening and closing a database connection is probably not a very good idea, as it is a (relatively) lengthy operation, incurs some overhead on the database server and might lead to a situation where the first half of your operation completes, but the second fails, because your reconnect to the database was unsuccessful.
1
  • Thnanks for answer. 1+
    – Sagotharan
    Commented Dec 29, 2011 at 12:24
2

Since you haven't declared any of these variables as static, your choice won't noticeably impact the application. It's simply a matter of coding-style, code organization, and ultimately readability.

Since the connection isn't subject to change during the application's lIfetime, I think it makes sense to make it static so that all class instances can share it.

On the other hand, command may vary across methods and datareader will surely vary per execution, so it makes sense to declare and instantiate those within the event handler methods.

2
  • 2
    I would be very careful about declaring things static, just to gain the convenience of not having to pass it to every instance. It is kind of the same reason why singletons are bad. Commented Dec 28, 2011 at 11:56
  • Thnanks for answer. 1+
    – Sagotharan
    Commented Dec 29, 2011 at 12:24
1

Generally you want to get the connection via a method from a source such as a config file. Within 1 transaction, you may want to use 1 connection. and you may also want to make sure that after a connection error, a message is logged or displayed and no further processing takes place. Connection errors are frequent during development and deployment and you may want to make sure they are identified separately from other rare errors.

There are many ways to write a code that satisfies your requirement, here is one way:

    string sqlconnection=getCnString(); //getConnection obtains and formats cn string
    using (SqlConnection connection = new SqlConnection(sqlconnection)
    {
    try
    {
         connection.Open();
         //...do stuff with the open connection

         // for command 1
         try
         {
         //...create a command and do stuff with it.
         }
         catch ...
         {
         // send message or log message then exit via finally block
         }
         // for command 2
         try
         {
         //...create a command and do stuff with it.
         }
         catch ...
         {
         // send message or log message then exit via finally block
         }
       }
    catch (Exception as ex)
    {

          MessageBox.Show("Connection open Error:"+ex.Message);
          //don't code return here

    }
    finally
    {
      connection.Close();
      connection.Dispose();
    }
  } //automatic dispose could take care of the connection implicitly as well
  return;
2
  • Thnanks for answer. 1+
    – Sagotharan
    Commented Dec 29, 2011 at 12:24
  • @Sagotharan, no problem
    – NoChance
    Commented Dec 29, 2011 at 15:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.