2
\$\begingroup\$

I wrote a program to copy a .doc template, add some text to it, then save and close it. I have to do this many times, and saving and closing a word document is slow. I decided to use multi-threading, but I'm a noob at this stuff and just wrote what came to me. Any feedback would be greatly appreciated (on any aspect, not just the multi-threading).

Here is my code for a task:

public Task
{
    public string Value;
    public bool HasRun;
    public Task(string value)
    {
        Value = value;
        HasRun = false;
    }
}

I create a list of Tasks that is a class level variable, accessible by all threads:

Tasks = db.table.select(a => new Task(a.value)).ToList();

Next, I run the following in each thread:

while(tasks.Any(a => a.HasRun == false))
{
    var value = "";
    lock(tasks)
    {
        var task = tasks.Where(a => a.HasRun == false).First();
        value = task.Value;
        task.HasRun = true;   
    }

    CreateNewDocumentAppendValueAndSave(value);
}
\$\endgroup\$

migrated from programmers.stackexchange.com Feb 16 '12 at 0:03

This question came from our site for professionals, academics, and students working within the systems development life cycle.

  • \$\begingroup\$ Are you sure you can't do this inside Word with MailMerge? It's not (even close to) limited to inserting addresses. If memory serves, you can use MailMerge to insert virtually anything from a database, for example. \$\endgroup\$ – Jerry Coffin Feb 15 '12 at 23:32
  • 2
    \$\begingroup\$ Instead of x == false, you can use !x. It's more clear in my opinion. \$\endgroup\$ – Ry- Feb 16 '12 at 0:13
  • 2
    \$\begingroup\$ Was your aim with multi threading to un-freeze the GUI of the app or was it to run the tasks faster by doing multiple tasks at the same time? \$\endgroup\$ – Alex Hope O'Connor Feb 16 '12 at 0:37
  • \$\begingroup\$ What version of .Net are you using? \$\endgroup\$ – svick Feb 16 '12 at 0:41
1
\$\begingroup\$

Let the framework do multithreading for you:

List<string> tasks = new List<string>();
foreach (string value in db.table.select(a => a.value))
{
    tasks.Add(Task.Factory.StartNew(() => CreateNewDocumentAppendValueAndSave(value)));
}
Task.WaitAll(tasks.ToArray());

Where Task is System.Threading.Tasks.Task.

\$\endgroup\$
  • \$\begingroup\$ This is very interesting, I was not aware of this method, thanks! \$\endgroup\$ – sooprise Feb 29 '12 at 15:59
0
\$\begingroup\$

Until we know your answers to Alex and svick's questions, here's a recommendation I have. I would change your loop slightly to this so that you only get the next task to run once per loop instead of twice.

I also normally name my variables used in LINQ expressions based on the variable being iterated through, so in this loop I've also renamed the a to t. I find it easier to read when the inner variable reflects what the outer variable is.

var task = tasks.FirstOrDefault(t => !t.HasRun);
while (task != null)
{
    var value = "";
    lock (tasks)
    {
        value = task.Value;
        task.HasRun = true;
    }

    CreateNewDocumentAppendValueAndSave(value);

    task = tasks.FirstOrDefault(t => !t.HasRun);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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