c# foreach loop for downloading image just works once

Question

I am creating a pc window application that downloads image from a website url. The user is allowed to select which folder to download to.

But the problem I have is that the foreach loop only does this process once...

public bool Url_checker(string link)
{
    try
    {
        //Creating the HttpWebRequest
        HttpWebRequest request = WebRequest.Create(link.Trim()) as HttpWebRequest;
        //Setting the Request method HEAD, you can also use GET too.
        request.Method = "HEAD";
        //Getting the Web Response.
        HttpWebResponse response = request.GetResponse() as HttpWebResponse;
        //Returns TRUE if the Status code == 200
        return (response.StatusCode == HttpStatusCode.OK);
    }
    catch (WebException)
    {
        return false;
    }
}

private void submit_Click(object sender, EventArgs e)
{
    FolderBrowserDialog folderBrowserDialog1 = new FolderBrowserDialog();
    folderBrowserDialog1.ShowDialog();
    string saveToThisFolder = folderBrowserDialog1.SelectedPath ;
    int i = 0;
    var di = new DirectoryInfo(saveToThisFolder);
    di.Attributes &= ~FileAttributes.ReadOnly;

    int counter = 0;
       foreach (string x in urllist.Lines)
       {
           string EndOfURL = x.Substring(x.Length - 4);
           if (Url_checker(x) && (EndOfURL == ".jpg" || EndOfURL == ".gif" || EndOfURL == ".jpeg" || EndOfURL == ".png"))
           {
              byte[] data;
             //using (WebClient client = new WebClient())
              //{
              WebClient client = new WebClient();
                  data = client.DownloadData(x);
             // }

              File.WriteAllBytes(saveToThisFolder + @"\" + (i++ + EndOfURL), data);
              counter++;
              workingURL.Text += x; //+ System.Environment.NewLine
           }
          else
          {

              errorURL.Text += (x + System.Environment.NewLine);
          }
       }


    folderBrowserDialog1.Dispose();
}

GUI design is here.

The last result is this,

It only prints and downloads once. Its as if, its only reading the 1st line of the textbox. Or The foreach loop is wrong.

Thanks

Here is the new CODE, CLeaned private void submit_Click(object sender, EventArgs e) {

        FolderBrowserDialog folderBrowserDialog1 = new FolderBrowserDialog();
        folderBrowserDialog1.ShowDialog();
        string saveToThisFolder = folderBrowserDialog1.SelectedPath;
        int i = 0;
        var di = new DirectoryInfo(saveToThisFolder);
        di.Attributes &= ~FileAttributes.ReadOnly;

        int counter = 0;
        foreach (string x in urllist.Lines)
        {
            try
            {
                string EndOfURL = x.Substring(x.Length - 4);
                if (Url_checker(x) && (EndOfURL == ".jpg" || EndOfURL == ".gif" || EndOfURL == ".jpeg" || EndOfURL == ".png"))
                {

                    byte[] data;
                    using (WebClient client = new WebClient())
                    {                            
                      data = client.DownloadData(x);
                      File.WriteAllBytes(saveToThisFolder + @"\" + ("MotivatinalQuoteImage" + (i++) + EndOfURL), data);
                      counter++;
                      workingURL.Text += x + System.Environment.NewLine;
                    }
                }
                else
                {
                    errorURL.Text += (x + System.Environment.NewLine);
                }
            }
            catch (Exception ex)
            {
                errorURL.Text += ex;
            }

        }
        folderBrowserDialog1.Dispose();
    }

So after debugging, I FOUND OUT THAT the mistake is SOMEWHERE between

                       byte[] data;
                       using (WebClient client = new WebClient())
                       {
                           data = client.DownloadData(x);
                           File.WriteAllBytes(saveToThisFolder + @"\" + ("MotivatinalQuoteImage" + (i++) + EndOfURL), data);
                           counter++;
                           workingURL.Text += x + System.Environment.NewLine;
                       } 

Here is what happened when I waited. After it only downloaded 1 image.

Answer 1

The problem is that this program only LOOPS through ONCE! However, without the code below. The FOR LOOP WORKS PERFECTLY. The code below is the problem.

byte[] data;
                        using (WebClient client = new WebClient())
                        {                            
                          data = client.DownloadData(x);
                          File.WriteAllBytes(saveToThisFolder + @"\" + ("MotivatinalQuoteImage" + (i++) + EndOfURL), data);
                          counter++;
                          workingURL.Text += x + System.Environment.NewLine;
                        }

Answer 2

I think that it's actually your Url_checker method returning false when it catches an exception.
When I test your code verbatim I get the same results, it only downloads the first image.
If I use the following Url_checker which disposes the HttpWebResponse each time it works.

    public bool Url_checker(string link)
    {
        try
        {
            HttpWebRequest request = WebRequest.Create(link.Trim()) as HttpWebRequest;
            request.Method = "HEAD";

            using (HttpWebResponse response = (HttpWebResponse)request.GetResponse())
            {
                return (response.StatusCode == HttpStatusCode.OK);
            }
        }
        catch (WebException)
        {
            return false;
        }
    }

This is because you must dispose of the response when it's done, otherwise the underlying infrastructure doesn't know what you did with the result and can't reuse the connection for the next response.

Edit: Can someone smarter than me clarify whether this is because the response is not being read in it's entirety so you must explicitly dispose it?

Edit 2: Never mind. From the documentation on HttpWebResponse:

https://msdn.microsoft.com/en-us/library/system.net.httpwebresponse%28v=vs.110%29.aspx

You should never directly create an instance of the HttpWebResponse class. Instead, use the instance returned by a call to HttpWebRequest.GetResponse. You must call either the Stream.Close or the HttpWebResponse.Close method to close the response and release the connection for reuse. It is not necessary to call both Stream.Close and HttpWebResponse.Close, but doing so does not cause an error.

Edit 3: Oops, turns out OP is also right. I habitually wrapped his WebClient in a using (which he had done originally) when testing and it works too. For the record you can also put a using against your FolderBrowserDialog rather than disposing it yourself, although either works.

byte[] data;
using (WebClient client = new WebClient())
{
    data = client.DownloadData(x);
}
File.WriteAllBytes(saveToThisFolder + @"\" + (i++ + EndOfURL), data);
counter++;
workingURL.Text += x + System.Environment.NewLine;

Answer 3

I mean

foreach (string x in urllist.Lines)
   {
try
{
       string EndOfURL = x.Substring(x.Length - 4);
       if (Url_checker(x) && (EndOfURL == ".jpg" || EndOfURL == ".gif" || EndOfURL == ".jpeg" || EndOfURL == ".png"))
       {
          byte[] data;
         //using (WebClient client = new WebClient())
          //{
          WebClient client = new WebClient();
              data = client.DownloadData(x);
         // }

          File.WriteAllBytes(saveToThisFolder + @"\" + (i++ + EndOfURL), data);
          counter++;
          workingURL.Text += x; //+ System.Environment.NewLine
       }
      else
      {

          errorURL.Text += (x + System.Environment.NewLine);
      }
   }
}
catch(Exception ex)
{//todo : add some logging here}

so you can check if there's an error on the SubString method

Answer 4

Try to put the content of foreach under try/catch... Starting from string EndOfUrl =....