Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have a class that will read from Excel (C# and .Net 4) and in that class I have a background worker that will load the data from Excel while the UI can remain responsive. My question is as follows: Is it bad design to have a background worker in a class? Should I create my class without it and use a background worker to operate on that class? I can't see any issues really of creating my class this way but then again I am a newbie so I figured I would make sure before I continue on.

I hope that this question is relevant here as I don't think it should be on stackoverflow as my code works, this just a design issue.

share|improve this question
3  
why do you think it might be wrong? –  Alb Feb 3 '11 at 22:35
1  
@Alb - it's hard to say. My code works and fills my needs, however, I'm planning on using this in a project that I will make open source. I want to make sure that my code doesn't "just work" and actually is well designed. –  Jetti Feb 4 '11 at 2:31

4 Answers 4

up vote 19 down vote accepted

Should I create my class without it and use a background worker to operate on that class?

Yes, you should. And I will tell you why - you are violating the Single Responsibility Principle. By tightly coupling the class that accesses the excel doc with how it accesses the excel doc, you eliminate the ability for the "controller" code (any code that uses this) to do it a different way. How different, you may ask? What if the controller code has two operations that take a long time but wants them to be sequential? If you allowed the controller the ability to handle the threading, it can do both long-running tasks together in one thread. What if you want to access the excel doc from a non-UI context and don't need it to be threaded?

By moving the responsibility of threading out to the caller, you allow more flexibility of your code, making it more reusable.

share|improve this answer
1  
+1 for a thoughtful explanation! –  Mike Brown Feb 3 '11 at 23:00
2  
+1 for answering the actual question that's asked and answering it well. –  Anna Lear Feb 3 '11 at 23:07
    
+1 - Thank you Nemi. You answered my question spot on, I appreciate it. I will pull that out of my class and put it in a new class, thank you again! –  Jetti Feb 4 '11 at 2:28
    
@Nemi - so would it be acceptable if I created another method that would would a synchronous load and then have an async method? Or would it just better to have the one synchronous load method and then go from there? –  Jetti Feb 4 '11 at 16:39
1  
I personally would not have a sync method and an async method. You are still coupling responsibilities. I have worked with this exact problem many times. What I did was created a TaskController that was modeled after SwingWorker, but had specific code for our app. The TaskController did things like update a progress bar, change the mouse cursor, etc. It may seem like boiler plate code to always have to create a TaskController to make calls, but ultimately the code is more robust and more maintainable. –  Nemi Feb 4 '11 at 17:04

It's good design to have UI operations operate in a seperate thread from background tasks. Otherwise the UI becomes unresponsive when the application is busy.

If you can separate the part that works in the background thread to its own class the code will be cleaner.

share|improve this answer
1  
Though correct, from how I read his question he does not have a problem understanding this point. –  Nemi Feb 3 '11 at 22:49
    
I'm sorry if my question was misread. I know that separating the UI and the background tasks is good design (if not necessary, my test excel file is over 8k rows and would make the program seem unresponsive). My question is basically is it good to tightly couple the thread with the Excel class. –  Jetti Feb 4 '11 at 2:32

I would separate your UI from your background task using separate classes. Doing so encourages separation of concerns. UI code and business logic should not be mixed.

share|improve this answer
    
Just to be clear, my class does not touch the UI. It has two events that would allow whatever is using it to update the UI if necessary, but my class in itself does not touch the UI. –  Jetti Feb 4 '11 at 16:37

From what I recall about BackgroundWorkers is that they provide a number of convienence methods like the ability to send progress updates to the UI. There's no rule that says you can't use it from a different class though.

Also if you're doing iteration that doesn't require items to be processed in a specific order, consider using the ThreadPool instead (or if you're on .NET 4 use the Task Parallel Library).

share|improve this answer
    
Thanks for your response. I created two events in my class that will triggers on progress (basically wrapping the BackgroundWorker progress event, as the BackgroundWorker is private) which sends progress to the UI (progress bar). –  Jetti Feb 4 '11 at 2:35

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.