Pergunta

Say I was to have a method that required a small number of objects to be created during each run, and the method would be called multiple times, i.e. a method to delete a given row from a database. Would it be better to create a new object each time and call the Garbage Collector (or similar) to destroy it at the end, or reinitialise the value each time?

Example:
Using new constructors each time:

private void RemoveFolder(string dir)
{
    OleDbCommand cmd2 = connection.CreateCommand();
    OleDbParameter parameterC = new OleDbParameter();
    cmd2.Parameters.Add(parameterC);
    parameterC.Value = dir;

    cmd2.CommandText = "DELETE * FROM [Directories] WHERE Path = ?";
    cmd2.ExecuteNonQuery();
    cmd2.Dispose();
}

Using a singular global variable (initialised within the constructor):

private void RemoveFolder(string dir)
{
    parameterC.Value = dir;

    cmd2.CommandText = "DELETE * FROM [Directories] WHERE Path = ?";
    cmd2.ExecuteNonQuery();
}

EDIT: When I say better, I mean "as-a-whole" during normal non-"mission-critical" programs, where a large increase in performance would trump a minor decrease in stability.

EDIT2

An example of calling a similar method multiple times

(Note that this is for another method of mine, AddFolder)

foreach (DirectoryInfo directory in directories)
{
    parameter.Value = directory.FullName;
    cmd.CommandText = "SELECT LastModified FROM Directories WHERE Path = ?";
    reader = cmd.ExecuteReader();

    while (reader.Read())
    {
        output += reader.GetString(0);
    }

    if (output == null)
    {
        Console.WriteLine("New Directory! " + directory.FullName);
        AddFolder(directory);
    }
    output = null;
    reader.Close();
 }
Foi útil?

Solução

Your question is Do I need to optimize that code?

My opinion in general,

  1. If it two easy to optimized it, and code still remain simple than do it, Otherwise if don't sure if it will give an impact than leave it that way,
  2. Measure it if you don't sure
  3. Leave comment near to the involved code

How to measure it? You can see if your program is running to slow, or if it using to much memory..

Outras dicas

Regarding to your second edit,

You can convert the foreach loop to LINQ, Aggregate all of the directories, and then at once Add the directories to you DB

Your code will look more elegant, and it will solve your main question

See http://www.linqtutorial.net/linqExamples.html there is paragraph how to replace iterative loop with LINQ

The major performance concern you have here is how much you pay for the "preparation" of the query. Preparation is where query gets parsed and (more importantly) query plan determined. The determination of query plan can be rather expensive and you want to minimize how many times this has to be done.

(NOTE: Even DELETE ... WHERE ... from your example needs a query plan.)

Some DBMSes and ADO.NET providers are better than others in "reusing" query plans. On Oracle/ODP.NET, for example, it probably doesn't matter if you repeatedly recreate your DbCommand - Oracle will tend to "see" that this is the same query that was used before and reuse the query plan.

On the other hand, if you want to ensure query is prepared once and reused multiple times, no matter what DBMS you use, it is not a bad idea to keep the same DbCommand object throughout your application execution (and even explicitly call DbCommand.Prepare), something like this:

var conn = new OracleConnection("your connection string"); // Or DB2Connection, SqlConnection or NpgsqlConnection or whatever...
conn.Open();

// Execute once:
var cmd = conn.CreateCommand();
cmd.CommandText = "DELETE FROM YOUR_TABLE WHERE YOUR_FIELD = :your_param";
var param = cmd.CreateParameter();
param.ParameterName = "your_param";
param.DbType = ...;
cmd.Parameters.Add(param);
cmd.Prepare(); // Not really important for Oracle, but may be important for others.

// Execute multiple times:
param.Value = "some value";
int row_count = cmd.ExecuteNonQuery();

// If you are concerned with long-lived connections, you'll typically be able to do this:
conn.Close();
// ...
conn.Open();
param.Value = "some other value";
row_count = cmd.ExecuteNonQuery();

// Etc..

So:

  • If your program only works on a single DBMS/ADO.NET provider and you know it will reuse query plans efficiently and you don't mind slightly more expensive C# code, you may go for the "repeated recreation" solution. It may also allow for a slightly shorter C# code.
  • However, I prefer "create once, reuse many times" solution - I'll get optimal performance whatever DBMS I use. And even under those DBMSes intelligent enough to reuse query plans, I'll still avoid re-execution of the C# code and associated garbage collection pressure.

I think the key piece of advice here is not to leave lingering database connections around, and then open and operate on an ad hoc basis. I notice that your question really revolves around whether to reuse a command or not - when, on looking, noticing your connection must already be reusable... I wouldn't recommend this and would suggest you think of each database operation as an atomic unit of work, including connection utilisation and all.

It is better to use the first method, so that the logic of executing the query stays in a single function.

I think the correct version is in between

public foo() 
{
   var context = generateContext(dir);
   context.excute();
}

Just split the two goals

  1. Creating the context
  2. Execute it

In the creating context use can use singleton

I would prefer using global variables and not recreating the same objects each operation. The one time allocation cost is minor, whereas creating the objects every iteration can be costly.

Licenciado em: CC-BY-SA com atribuição
Não afiliado a StackOverflow
scroll top