That code does not make much sense... and it is not maintainable.
Some comments on the code. Lot more issues can be found:
1)If you use background workers, you probably don't need explicit
Invoke
. Use
ReportProgress
instead.
2) If you have more than 5 rows in your data, some row might not be processed as you do nothing if all worker are busy.
3) In your workers, you set
i
to
1
in each iteration. This will create an "infinite" loop but in a very obscur way. If you want an infinite loop, then don't use variable
i
.
4) It does not make much sense to report on
i
(commented out code), if the value is always 2 except the first one.
5) Spelling error in recived.
6) Way too much duplicate code. Have you ever read on DRY?
Don't repeat yourself - Wikipedia, the free encyclopedia[
^]
Don't Repeat Yourself - Programmer 97-things[
^]
Dont Repeat Yourself[
^]
7)Your functions are too complicate. You should read articles on SRP.
Single responsibility principle - Wikipedia, the free encyclopedia[
^]
SOLID: Part 1 - The Single Responsibility Principle[
^]
Single Responsibility Principle | Object Oriented Design[
^]
SOLID Principles: Single Responsibility Principle -> What, Why and How.[
^]
8) Avoid hard-coded constants in code. You have a bunch of them in your code:
500
(maximum number of items in list)
1
(some server status)
"SG_LeadServer"
(hard-coded string)
At the very minimum, put constants at class level so you have single and easy to find place to find them and only one value to update if you change your mind.
9) Avoid useless comments like this one:
//Issue a cancellation request to stop the background worker
. Comment should not repeat the (obvious) code (except for teaching purpose in school book).
10) If you have 5 threads, then it does not make sense that all of them update the same status. You should probably display a cumulative status in your case.
11) Delay in background worker is often suspicious. Why would you want to slow down processing.
12) You should remove obsolete code or put it inside
#if obsolete_code / #endif
or at least add a comment about why the code was commented out. In a forum such unused code is not really helpful although it give us some hint about what you try but we still don't know why it was commented out.
13) I don't see the purpose of the lock (code that use it is commented out). As you are calling
Invoke
from the
DoWork
handler, you already know that a single thread will be executing that code at the same time (assuming no other thread directly call the code --- in which case, the code would fails anyway because of cross-thread calls to UI).
14) Even worst is the fact that the code is hard-code to work with 5 workers. If one want to try with more or less workers to see the effect on performance, it is hard to adapt the code.
15) You should avoid mixing non UI code and UI code in the same class. This is even more the case here where code for each worker is more or less a copy of the code of other workers. In such case, you should have at least 2 classes... Code in the UI should be minimalist. If you ever want to port code on other platform (iOS, ASP.NET, Android, UWP, WPF,...) you want to share as much as possible code.