We help IT Professionals succeed at work.

Which is Better Code MySQL CSharp Parameter

rctems
rctems asked
on
1,009 Views
Last Modified: 2012-08-13
Which is Better code?

       private void LogMessage(string Message)
        {
            this.LogMessageL(Message);
            if (mySqlConnection1 == null)
            {
                mySqlConnection1 = new MySqlConnection();
                this.mySqlConnection1.ConnectionString = Properties.Settings.Default.MySQL_ConnectionString;

            }
            try
            {
                MySqlCommand cmd = new MySqlCommand("sp_service_log_nightlybatch", mySqlConnection1);
                cmd.Parameters.Clear();
                cmd.CommandType = CommandType.StoredProcedure;
                cmd.Parameters.Add(new MySqlParameter("LOGID", MySqlDbType.UInt32));
                cmd.Parameters.Add(new MySqlParameter("STATME", MySqlDbType.LongText));
                cmd.Parameters.Add(new MySqlParameter("DATECREATED", MySqlDbType.VarChar, 20));
                cmd.Parameters[0].Value = 1;
                cmd.Parameters[1].Value = Message.Replace('\'',' ');
                cmd.Parameters[2].Value = DateTime.Now.ToString("yyyyMMddHHmmss");
                cmd.UpdatedRowSource = UpdateRowSource.None;
                if (mySqlConnection1.State.Equals(ConnectionState.Closed))
                    mySqlConnection1.Open();
                cmd.ExecuteNonQuery();
                cmd.Dispose();
            }
            catch (Exception ex)
            {
                this.LogMessageL(ex.Message);
            }
            finally
            {
                mySqlConnection1.Dispose();
            }
        }

or


        private void LogMessage2(string Message)
        {
            this.LogMessageL(Message);
            if (mySqlConnection1 == null)
            {
                mySqlConnection1 = new MySqlConnection();
                this.mySqlConnection1.ConnectionString = Properties.Settings.Default.MySQL_ConnectionString;

            }
            try
            {
                MySqlCommand cmd = new MySqlCommand("sp_service_log_nightlybatch", mySqlConnection1);
                cmd.Parameters.Clear();
                cmd.CommandType = CommandType.StoredProcedure;
                cmd.Parameters.AddWithValue("LOGID",1);
                cmd.Parameters.AddWithValue("STATME",Message.Replace('\'', ' '));
                cmd.Parameters.AddWithValue("DATECREATED", DateTime.Now.ToString("yyyyMMddHHmmss"));
                cmd.Parameters["LOGID"].MySqlDbType = MySqlDbType.UInt32;
                cmd.Parameters["STATME"].MySqlDbType = MySqlDbType.LongText;
                cmd.Parameters["DATECREATED"].MySqlDbType = MySqlDbType.VarChar;
                cmd.Parameters["DATECREATED"].Size = 20;
                cmd.Parameters["LOGID"].Direction = ParameterDirection.Input;
                cmd.Parameters["STATME"].Direction = ParameterDirection.Input;
                cmd.Parameters["DATECREATED"].Direction = ParameterDirection.Input;
                cmd.UpdatedRowSource = UpdateRowSource.None;
                if (mySqlConnection1.State.Equals(ConnectionState.Closed))
                    mySqlConnection1.Open();
                cmd.ExecuteNonQuery();
                cmd.Dispose();
            }
            catch (Exception ex)
            {
                this.LogMessageL(ex.Message);
            }
            finally
            {
                mySqlConnection1.Dispose();
            }
        }
Comment
Watch Question

Commented:
private void LogMessage(string Message)
        {
            this.LogMessageL(Message);
            if (mySqlConnection1 == null)
            {
                mySqlConnection1 = new MySqlConnection();
                this.mySqlConnection1.ConnectionString = Properties.Settings.Default.MySQL_ConnectionString;

            }
            try
            {
                MySqlCommand cmd = new MySqlCommand("sp_service_log_nightlybatch", mySqlConnection1);
                cmd.Parameters.Clear();
                cmd.CommandType = CommandType.StoredProcedure;
                cmd.Parameters.Add(new MySqlParameter("LOGID", MySqlDbType.UInt32)).Value = 1;
                cmd.Parameters.Add(new MySqlParameter("STATME", MySqlDbType.LongText)).Value = Message;
                cmd.Parameters.Add(new MySqlParameter("DATECREATED", MySqlDbType.VarChar, 20)).Value = DateTime.Now.ToString("yyyyMMddHHmmss");
                if (mySqlConnection1.State.Equals(ConnectionState.Closed))
                    mySqlConnection1.Open();
                cmd.ExecuteNonQuery();
                cmd.Dispose();
            }
            catch (Exception ex)
            {
                this.LogMessageL(ex.Message);
            }
            finally
            {
                mySqlConnection1.Dispose();
            }
        }

reduce the line of code :D

Author

Commented:
Possibly

But Apparently the .add function is deprecated ie it is nolonger in use and some people are suggesting AddWithValue is prefered.

I'm trying to make a reasoned rational argument. Less lines is better but depreicated code introduces a quality aspect.

Ryan
Top Expert 2007

Commented:
Personally, I would not use either one. What you have here ties an awful lot of database-specific and database provider-specific code to the execution of a single stored procedure. What if you have 50 of them? What if you have a large application and several hundred selects to support against different tables and views, each with different numbers and types of parameters?What if you write all that code, and you are told you now have to support Oracle on the back end?

Here's an alternative. Create a helper class for each provider that creates provider-specific objects. The caller sends requests through to whichever provider you are using today to construct appropriate parameters. Then it sends in a SQL string, or in this case a procedure name, and this method performs the actual database-specific activity.

public void InvokeStoredProc( string procName, ArrayList spParams )
{
   int paramCount, index;

   IDataParameterCollection parameters = mIParamCollection;
   parameters.Clear();

   paramCount = spParams.Count;
   index = 0;
   while (index < paramCount)
   {
      parameters.Add(spParams[index++]);
   }
   mICommand.CommandType = CommandType.StoredProcedure;
   mICommand.CommandText = procName;
   mICommand.Connection = mIConnection;

   OpenConnection();
   if (mInTransaction)
   {
      mICommand.Transaction=mITransaction;
   }
   try
   {
      mICommand.ExecuteNonQuery();
      if (!mInTransaction)
      {
         CloseConnection();
      }
   }
   catch(Exception ex)
   {
      if (!mInTransaction)
      {
         CloseConnection();
      }
      throw new Exception("Error: The procedure:\n" + procName + "\nCaused this exception: " + ex.Message, ex);
   }
}

All this uses references to the interfaces that any provider-specific objects must implement. I can stop my application, change the connection string, and switch from Oracle to SQL Server to anything else I need without changing a single line of code.

Jim

Author

Commented:
Wow,

This is a completely new, and very smart ..... I like it for a lot of reasons

I'm battling to find a reason not to rewrite all my code using this idea.

Ryan
Top Expert 2007

Commented:
I was lucky - I thought of it when we first started making the transition from C++ to C#. I knew I had to support both Oracle and SQL Server, and did not want to have two sets of everything. There is one drawback - except for stored procedures, you need to beat anyone over the head who writes non-ANSI SQL.

I also had to create a few functions for simple things. For example, SQL Server uses '+' for string concatenation, while Oracle uses '||' (which is ANSI). So I had to create a CONCATENATE() function and rewrite a handful of SQL statements.

Jim
Top Expert 2007

Commented:
I also recently added the ability to trace all our database accesses by adding a few lines to about 15 methods. I pulled it out before to avoid confusion. We build a large MES (OLTP) system used by Fortune 100 companies. It's a web app, and can have several thousand concurrent users. All our screens are built dynamically, so I have to keep a close watch on DB access and be able to easily track down and remove any that are unnecessary, or fix any that get ugly or inefficient. Here's the version of that method with the trace code in.

public void InvokeStoredProc( string procName, ArrayList spParams )
{
   if (mLogger != null)
   {
      mTimer.Reset();
      mTimer.Start();
   }
   int paramCount, index;

   IDataParameterCollection parameters = mIParamCollection;
   parameters.Clear();

   paramCount = spParams.Count;
   index = 0;
   while (index < paramCount)
   {
      parameters.Add(spParams[index++]);
   }
   mICommand.CommandType = CommandType.StoredProcedure;
   mICommand.CommandText = procName;
   mICommand.Connection = mIConnection;

   OpenConnection();
   if (mInTransaction)
   {
      mICommand.Transaction=mITransaction;
   }
   try
   {
      mICommand.ExecuteNonQuery();
      if (!mInTransaction)
      {
         CloseConnection();
      }
   }
   catch(Exception ex)
   {
      if (!mInTransaction)
      {
         CloseConnection();
      }
      if (mLogger != null)
      {
         mTimer.Stop();
         StringBuilder sb = new StringBuilder(2048);
         sb.Append("<Method>InvokeStoredProc</Method><SQL>" + procName + "</SQL><Parameters>" + DumpParameters(spParams) + "</Parameters><Error>" + ex.Message + "</Error><ElapsedTime>" + mTimer.ElapsedMilliseconds.ToString() + "</ElapsedTime>");
         mLogger.Debug(sb.ToString());
      }
      throw new Exception("Error: The procedure:\n" + procName + "\nCaused this exception: " + ex.Message, ex);
   }

   if (mLogger != null)
   {
      mTimer.Stop();
      StringBuilder sb = new StringBuilder(2048);
      sb.Append("<Method>InvokeStoredProc</Method><SQL>" + procName + "</SQL><Parameters>" + DumpParameters(spParams) + "</Parameters><Error></Error><ElapsedTime>" + mTimer.ElapsedMilliseconds.ToString() + "</ElapsedTime>");
      mLogger.Debug(sb.ToString());
   }
}

The logger also appends the call stack, so it's easy to trace where where the whole thing originated. I wrapped each of the logged items in XML-like tags, and wrote a windows app to let me view and search the log files. It has been a life-saver for me. I also have the ability to turn it on at a customer site for a single user to help track data-dependent bugs.

The logger we use is called NLog. You can check it out here if you are interested.

http://www.nlog-project.org/

Jim

I highly recommend something like that to help you keep track of things.

Author

Commented:
Awsome,

I dream about that .. but the cold hard reality is that my company doesn't allow me the freedom to impliment labour saving fault finding technology as this because they view it as a 'waste of time' and too much code rewriting.(Accountants <shrug>) Little do they know that error logging saves time and money!

*sigh*

Ryan
Top Expert 2007

Commented:
I'm lucky - I started my own company, so I pull a bit more weight with the bean counters. However, it shouldn't be difficult to justify over time anywhere. Did I mention that NLog is free?

Jim

Author

Commented:
J-

I mentioned it but boss says "low priority". He seams to think that programing only consists of coding and doesn't seam to comprehend that testing is equally if not more important! The ability to solve those hard bugs is determined by the quality of information used to frame questions. And that information is only availalbe from automated, comphrehensive testing.   How do I explain that automated testing software saves money, not costs money!

He seams to think that perfect code just happens and that testing is not necessary.

*sigh*

-R
Top Expert 2007
Commented:
Unlock this solution and get a sample of our free trial.
(No credit card required)
UNLOCK SOLUTION

Author

Commented:
J-

Your company sounds great.

-R

Gain unlimited access to on-demand training courses with an Experts Exchange subscription.

Get Access
Why Experts Exchange?

Experts Exchange always has the answer, or at the least points me in the correct direction! It is like having another employee that is extremely experienced.

Jim Murphy
Programmer at Smart IT Solutions

When asked, what has been your best career decision?

Deciding to stick with EE.

Mohamed Asif
Technical Department Head

Being involved with EE helped me to grow personally and professionally.

Carl Webster
CTP, Sr Infrastructure Consultant
Empower Your Career
Did You Know?

We've partnered with two important charities to provide clean water and computer science education to those who need it most. READ MORE

Ask ANY Question

Connect with Certified Experts to gain insight and support on specific technology challenges including:

  • Troubleshooting
  • Research
  • Professional Opinions
Unlock the solution to this question.
Thanks for using Experts Exchange.

Please provide your email to receive a sample view!

*This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.

OR

Please enter a first name

Please enter a last name

8+ characters (letters, numbers, and a symbol)

By clicking, you agree to the Terms of Use and Privacy Policy.