Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

In my program I read several information's about the system. I read each categories like CPU, memory, services, software and so on with a own thread. The main program have to wait for the longest thread.

To implement this, I use LINQ and the Join function from Thread.

Here is a small part from my code:

var culture = CultureInfo.CreateSpecificCulture("en-US");
var thread_list = new List<Thread>();
var c = new Computer();

var thrma = new Thread(() =>
{
    c.Mainboard = new Mainboard().GetInformation();
});
thrma.CurrentUICulture = culture;
thrma.Start();
thread_list.Add(thrma);

var thrce = new Thread(() =>
{
    c.ProcessorList = new CentralProcessingUnit().GetInformation();
});
thrce.CurrentUICulture = culture;
thrce.Start();
thread_list.Add(thrce);

...

var thrsvr = new Thread(() =>
{
    DeviceService.ScanServices();
});
thrsvr.CurrentUICulture = culture;
thrsvr.Start();
thread_list.Add(thrsvr);

foreach (Thread t in thread_list)
{
    t.Join();
}

// Everyone is done!

Each hardware device is derived from the Hardware class.

What do you think about the solution? Is there a "nicer" or better implementation? I would like to use a function, because the code is very similar, but I'm not sure how to do it. Any idea?

Thanks for your advices.

share|improve this question
Which .NET version are you using? Are these Mainboard, CentralProcessingUnit and DeviceService classes written by you or came from 3rd-party library? – almaz May 22 at 14:57
I use .Net 4. I wrote those classes. – hofmeister May 22 at 15:01
I have forgotten to say, that the threads are running in a services. – hofmeister May 22 at 15:48

1 Answer

up vote 1 down vote accepted

It's better to use new asynchronous API since it provides more features for combining asynchronous tasks.

Ideally Mainboard, CentralProcessingUnit and DeviceService should expose asynchronous API if a certain operation is not trivial, so (as you mentioned in comments you wrote them) I would implement asynchronous API for them first, e.g.:

public class Mainboard
{
    public Task<string> GetInformationAsync()
    {
        //TODO: extract long-term operation into Task. Try not to span extra threads here
    }
}

Then your method would look like:

Task.WaitAll(
    new Mainboard().GetInformationAsync().ContinueWith(task => c.Mainboard = task.Result),
    new CentralProcessingUnit().GetInformationAsync().ContinueWith(task => c.ProcessorList = task.Result),
    DeviceService.ScanServicesAsync());

If you decide not to implement async API the code can look like:

var c = new Computer();

Task.WaitAll(
    Task.Factory.StartNew(() => c.Mainboard = new Mainboard().GetInformation()),
    Task.Factory.StartNew(() => c.ProcessorList = new CentralProcessingUnit().GetInformation()),
    Task.Factory.StartNew(() => DeviceService.ScanServices()));
share|improve this answer
Looks very nice! Is it possible to set the CultureInfo in the Task. The asynchronous API doesn't block the main program, right? Thanks! – hofmeister May 22 at 15:33
I would rather explicitly pass CultureInfo to methods that require it instead of relying on current thread CultureInfo. – almaz May 22 at 16:01

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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