C# } For Loop Issues

Question

Hello so my school has records that i needed to access but i forgot my user id, so i made a program that would download any possible PDF and search them for my name. If it found mine it would alert me however when the program was down and said "We found a file containing Sean File Name is : xxxxxx.pdf" The actual number part would be off by however much the for loop is incremented by even though this was on a separate method! BTW School name will be starred out!

*B y the way! In Search() I used i-10 as a temporary fix, but I was thinking their must be a better way *

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Net;
using BitMiracle.Docotic.Pdf; // pdf parser / viewer
using System.IO;
using System.Threading;

namespace XXXXXXX_TEMP
{
    class Program
{
    static void Main(string[] args)
    {
        Program p1 = new Program();
        p1.Start();
    }

    private void Start()
    {
        WebClient wc = new WebClient();
        for (int i = 200000; i < 999999999; i = i + 10)
        {
            try
            {
                wc.DownloadFile("XXXXXXXXXXXXXXXXXXXXXX/" + i + ".pdf", i + ".pdf");
            }
            catch (WebException) {
                continue;
            }
            PdfDocument pdf = new PdfDocument(i + ".pdf");
            Thread a = new Thread(() => Search(i, pdf));
            a.Start();

            if (i == 999999) {
                Console.ReadLine();
            }
        }
    }

    private void Search(int i, PdfDocument pdf)
    {
        i = i - 10;
        String html = pdf.GetText();
        if (html.ToLower().Contains("sean"))
        {
            Console.WriteLine("Found File Containing Sean! File Name Is : " + i + ".pdf\n");
            Console.WriteLine("PDF Text = " + html);
      }
    }
  }
}

This very common mistake is addressed in a number of other Stack Overflow Q&A already, including the marked duplicate. See in particular [this answer](http://stackoverflow.com/a/3157918) for a good explanation. — Peter Duniho, Jul 22 '16 at 04:11
@PeterDuniho Very Sorry, This happens to me often as I have issues wording things. Ill be much more careful next time sorry. — Sean da Potato, Jul 22 '16 at 04:45

score 0 · Accepted Answer · edited May 23 '17 at 11:58

0

What you've run into is called closing over the loop variable. You should carefully study the linked answer as well as the blog entry it links to.

The immediate solution to your problem is to pass a copy of i to the thread. That is:

int index = i;
Thread a = new Thread(() => Search(index, pdf));

However, you might find that starting all those threads seriously bogs down your system, and leaving all those threads hanging could potentially eat up resources and lead to an out of memory exception. A cleaner way to do it is to call ThreadPool.QueueUserWorkItem, like this:

ThreadPool.QueueUserWorkItem(o => Search(index, pdf));

Or, in newer versions of .NET:

Task.Run(() => Search(index, pdf));

See Simplest way to do a fire and forget method in C#? for more information about fire-and-forget threads.

edited May 23 '17 at 11:58

Community

1
1

answered Jul 22 '16 at 03:52

Jim Mischel

131,090
20
188
351

@SeandaPotato: The primary point is that your solution is *incorrect* because it can skip some documents and process others multiple times. Any speed difference will be on the order of microseconds. The `i-10` thing is not reliable. – Jim Mischel Jul 22 '16 at 04:15
Ah okay. One more question so what I understood from looking MSDN is that Task.Run() is the new and easier way of Task.Facotry.StartNew(), but it does restrict the user from being able to do certain things. The one think confusing to me about Task.Run is that it says it runs what you send it off of the thread pool. Do I need to make this thread pool, or is it already there. Also with ThreadPool.QueueUserWorkItem(o => Search(index, pdf)); do I need to make a thread pool here to or yet again is it handled by the system? – Sean da Potato Jul 22 '16 at 04:37
@SeandaPotato: The thread pool is created and managed automatically by the system. See https://msdn.microsoft.com/en-us/library/0ka9477y(v=vs.110).aspx for more information. – Jim Mischel Jul 22 '16 at 05:26

C# } For Loop Issues

1 Answers1