My DoWork event code is not built good. So many times im calling the downloadContent() can someone hlep?

Question

I wrote this downloadContent() function.
Is there some other way to download the content faster?

private string downloadContent() 
{
    try
    {
        WebRequest request = WebRequest.Create(testingUrl);
        request.Proxy = null;
        request.Method = "GET";
        response = request.GetResponse();
        Stream stream = response.GetResponseStream();
        reader = new StreamReader(stream);
        string content = reader.ReadToEnd();
        return content;
    }
    catch
    {
        return error;
    }
}

Now this is the two functions im using to extract the text from in between tags after downloading:

private void GetProfileNames(string text)
{
    names = new List<string>();
    string startTag = "<span class=\"message-profile-name\" ><a  href='/profile/";
    string endTag = "'>";
    int startTagWidth = startTag.Length;
    int endTagWidth = endTag.Length;
    index = 0;
    while (true)
    {
        index = text.IndexOf(startTag, index);
        if (index == -1)
        {
            break;
        }
        // else more to do - index now is positioned at first character of startTag 
        int start = index + startTagWidth;
        index = text.IndexOf(endTag, start + 1);
        if (index == -1)
        {
            break;
        }
        // found the endTag 
        profileName = text.Substring(start, index - start);
        names.Add(profileName);
    }
}

And this is the DoWork() event which is not good the way I did it, but I just don't know how to make it better:

private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e) 
{
    string templastItem;
    BackgroundWorker worker = sender as BackgroundWorker; 
    List<string> tempNamesAndTexts = new List<string>();
    string tempDownload = downloadContent();
    if (tempDownload == error)
    {
        if (InvokeRequired)
        {
            BeginInvoke(new Action(() => textBox1.AppendText(DateTime.Now + "===> " + "Error The Web Site Is Not Responding Wait, Trying To Reconnect" + Environment.NewLine)));
            Logger.Write("Error The Web Site Was Not Responding");
        }
    }
    else
    {
        GetProfileNames(tempDownload);
        GetTextFromProfile(tempDownload);
        for (int i = 0; i < names.Count; i++)
        {
            tempNamesAndTexts.Add(names[i] + " " + texts[i]);

        }
        templastItem = tempNamesAndTexts.Last();
        if (InvokeRequired)
        {
            BeginInvoke(new Action(() => tempNamesAndTexts.ForEach(Item => textBox1.AppendText(DateTime.Now + "===> " + Item + Environment.NewLine))));
            string[] array = File.ReadAllLines(full_path_log_file_name);
            if (array.Length == 2)
            {
                foreach (string item in tempNamesAndTexts)
                {
                    Logger.Write(item);
                }
            }
            else
            {
                if (!array.Contains(templastItem))
                {
                    Logger.Write(templastItem);
                }
            }

        }
    }
    while (true)
    {
        testingDownload = downloadContent();
        if (testingDownload == error)
        {
            if (InvokeRequired)
            {
                BeginInvoke(new Action(() => textBox1.AppendText(DateTime.Now + "===> " + "Error The Web Site Is Not Responding Wait, Trying To Reconnect" + Environment.NewLine)));
                Logger.Write("Error The Web Site Was Not Responding");
            }
            Thread.Sleep(1000);

        }
        else
        {
            Logger.exist();
            namesAndTexts = new List<string>();
            if ((worker.CancellationPending == true))
            {
                e.Cancel = true;
                break;
            }
            else
            {



                string content = downloadContent();
                if (content == error)
                {
                    if (InvokeRequired)
                    {
                        BeginInvoke(new Action(() => textBox1.AppendText(DateTime.Now + "===> " + "Error The Web Site Is Not Responding Wait, Trying To Reconnect" + Environment.NewLine)));
                        Logger.Write("Error The Web Site Was Not Responding");
                    }
                    Thread.Sleep(1000);
                }
                else
                {

                    GetProfileNames(content);
                    GetTextFromProfile(content);
                    for (int i = 0; i < names.Count; i++)
                    {
                        namesAndTexts.Add(names[i] + " " + texts[i]);

                    }
                    if (InvokeRequired)
                    {
                        result = tempNamesAndTexts.SequenceEqual(namesAndTexts);
                        if (result == true)
                        {
                        }
                        else
                        {
                            var t = namesAndTexts.Last();


                            if (textBox1.InvokeRequired)
                            {

                                BeginInvoke(new Action(() => textBox1.AppendText(DateTime.Now + "===> " + t + Environment.NewLine)), null);
                                Logger.Write(t);
                                if (result == false)
                                {
                                    tempNamesAndTexts = new List<string>();
                                    for (int i = 0; i < names.Count; i++)
                                    {

                                        tempNamesAndTexts.Add(names[i] + " " + texts[i]);
                                    }
                                }



                            }
                        }


                    }
                    reader.Close();
                    response.Close();
                    Thread.Sleep(1000);
                }
            }
        }
    }

}

private void GetTextFromProfile(string text)
{
    texts = new List<string>();
    string str = "<span class=\"message-text\">";
    string startTag = str;
    string endTag = "</span>";
    int startTagWidth = startTag.Length;
    int endTagWidth = endTag.Length;
    index = 0;
    while (true)
    {
        index = text.IndexOf(startTag, index);
        if (index == -1)
        {
            break;
        }
        // else more to do - index now is positioned at first character of startTag 
        int start = index + startTagWidth;
        index = text.IndexOf(endTag, start + 1);
        if (index == -1)
        {
            break;
        }
        // found the endTag 
        profileNameText = text.Substring(start, index - start);
        Conditions();


    }
}

And this is the link to view/download the project, which is not big:

https://skydrive.live.com/redir?resid=3B8A7D9F66FF985B!171&authkey=!AFO6EmoF38MtkKQ

Answer 1

I don't know about faster, but please make sure you wrap your disposable resources (classes which implement IDisposable) in using statements to properly and deterministically manage resources:

    private string downloadContent() 
    {
        try
        {
            WebRequest request = WebRequest.Create(testingUrl);
            request.Proxy = null;
            request.Method = "GET";
            using (var response = request.GetResponse())
            using (var stream = response.GetResponseStream())
            using (var reader = new StreamReader(stream))
            {
                return reader.ReadToEnd();
            }
        }
        catch
        {
            return error;
        }
    }

Answer 2

First off, I like the use of white space and the layout.

On top of what Jesse said here's my $0.02

I think you're using too many class variables in this code. You also have way too many nested ifs. I have noted below where you should be removing the nesting and why it is not needed.

Start in the GetProfileNames method. The while the loop works, but it is very messy, and the while(true) statement really confuses the code. I would try something like this, it is cleaner, and probably more efficient

private IEnumerable<string> GetProfileNames(string text)
{
    const string startTag = "<span class=\"message-profile-name\" ><a  href='/profile/";
    const string endTag = "'>";

    var names = text.Split(new[] {startTag, endTag}, StringSplitOptions.RemoveEmptyEntries);

    return names;
}

Modify your Conditions method to be something a little more descriptive. You should change the signature too so you can pass in the string you're processing. This will clarify what call is supposed to be doing.

private void ProcessProfileNameTexts(string profileNameText)
{
    ...
}

You can then apply the same logic as you did in GetProfileNames to GetTextFromProfile

private IEnumerable<string> GetTextFromProfile(string text)
{

    const string startTag = "<span class=\"message-text\">";
    const string endTag = "</span>";

    var texts = text.Split(new[] { startTag, endTag }, StringSplitOptions.RemoveEmptyEntries).ToList();

    texts.ForEach(ProcessProfileNameTexts);

    return texts;
}

Your DoWork method is a mess. I would start by adding a return to the if of the error check and removing the else. I would also stop using the '+' to concatenate strings, this is a very inefficient way of doing it. My suggestion would be to use string.Format()

First off, add a few helper methods

private static void Invoke(string message)
{
    BeginInvoke(new Action(() => textBox1.AppendText(string.Format(@"{0} ===> {1}", DateTime.Now, message))));
}

private static void Log(string message)
{
    Logger.Write(message);
}

private static void InvokeAndLogIfRequired(string message, string loggerMessage)
{
    if (!InvokeRequired)
    {
        return;
    }

    Invoke(message);
    Log(loggerMessage);
}

Then change your error check to

if (tempDownload == error)
{

    InvokeAndLogIfRequired(
        string.Format(@"Error The Web Site Is Not Responding Wait, Trying To Reconnect{0}", Environment.NewLine), 
        "Error The Web Site Was Not Responding");


    return;
}

Your method lists can then be assigned:

names = GetProfileNames(tempDownload);
texts = GetTextFromProfile(tempDownload);

var tempNamesAndTexts = new List<string>();
for (var i = 0; i < names.Count; i++)
{
    tempNamesAndTexts .Add(string.Format(@"{0} {1}", names[i], texts[i]));
}

The variable templastItem should be declared as it is assigned

var templastItem = tempNamesAndTexts.Last();

This next section is not as clean as I'd like, but it is a little cleaner that the original code.

if (InvokeRequired)
{
    tempNamesAndTexts.ForEach(Invoke);

    var array = File.ReadAllLines(full_path_log_file_name);
    if (array.Length == 2)
    {
        tempNamesAndTexts .ForEach(Log);
    }
    else
    {
        if (!array.Contains(templastItem))
        {
            Log(templastItem);
        }
    }
}

I'm not going to take out the next while loop because I don't think I have that much brain power right now, but lets clean it up:

Make the error reporting a little cleaner but using helper methods and adding a continue instead of the else statement

testingDownload = downloadContent();
if (testingDownload == error)
{
    InvokeAndLogIfRequired("Error The Web Site Is Not Responding Wait, Trying To Reconnect", "Error The Web Site Was Not Responding");

    Thread.Sleep(1000);

    continue;
}

Remove the else from the cancel pending code too. The break takes care of the else.

Logger.exist();
var namesAndTexts = new List<string>();
if ((worker.CancellationPending == true))
{
    e.Cancel = true;
    break;
}

Clean up the next error checking:

string content = downloadContent();
if (content == error)
{
    InvokeAndLogIfRequired(
        string.Format(@"Error The Web Site Is Not Responding Wait, Trying To Reconnect{0}", Environment.NewLine),
        "Error The Web Site Was Not Responding");

    Thread.Sleep(1000);

    continue;
}

Now comes to the last Invoke, going to start after the if statement:

The if here is useless. Check for false and continue through the false path.

result = tempNamesAndTexts.SequenceEqual(namesAndTexts);
if (result == true)
{
}

Like this:

result = tempNamesAndTexts.SequenceEqual(namesAndTexts);
if (result == false)
{
    ...
}

This next section is very confusing, within the

if (InvokeRequired) 

you have

if (textBox1.InvokeRequired)

I'm assuming this is the same thing. If it is, the second one will ALWAYS be true.

you also have a

if (result == false)

inside a

if (result == false)

again, the second one will ALWAYS be false, remove it.

These suggestions just scratch the surface, there is much more I could do to this, but this is a good start.

Here is the entire DoWork method as I've refactored it, I haven't tested it, but it should work.

private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
{
    var worker = sender as BackgroundWorker;

    string tempDownload = downloadContent();
    if (tempDownload == error)
    {
        InvokeIfRequires(
            string.Format(@"Error The Web Site Is Not Responding Wait, Trying To Reconnect{0}",
                            Environment.NewLine),
            "Error The Web Site Was Not Responding");


        return;
    }

    names = GetProfileNames(tempDownload);
    texts = GetTextFromProfile(tempDownload);
    var tempNamesAndTexts = new List<string>();
    for (var i = 0; i < names.Count; i++)
    {
        tempNamesAndTexts.Add(string.Format(@"{0} {1}", names[i], texts[i]));
    }

    var templastItem = tempNamesAndTexts.Last();

    if (InvokeRequired)
    {
        tempNamesAndTexts.ForEach(Invoke);

        var array = File.ReadAllLines(full_path_log_file_name);
        if (array.Length == 2)
        {
            tempNamesAndTexts.ForEach(Log);
        }
        else
        {
            if (!array.Contains(templastItem))
            {
                Log(templastItem);
            }
        }
    }

    while (true)
    {
        testingDownload = downloadContent();
        if (testingDownload == error)
        {
            InvokeAndLogIfRequired("Error The Web Site Is Not Responding Wait, Trying To Reconnect",
                                    "Error The Web Site Was Not Responding");

            Thread.Sleep(1000);

            // Maybe put a count in here so you don't end up in an infinite loop of the web server is down
            continue;
        }

        Logger.exist();

        if ((worker.CancellationPending == true))
        {
            e.Cancel = true;
            break;
        }

        string content = downloadContent();
        if (content == error)
        {
            InvokeAndLogIfRequired(
                string.Format(@"Error The Web Site Is Not Responding Wait, Trying To Reconnect{0}",
                                Environment.NewLine),
                "Error The Web Site Was Not Responding");

            Thread.Sleep(1000);

            // Maybe put a count in here so you don't end up in an infinite loop of the web server is down


            continue;
        }


        var namesAndTexts = new List<string>();
        names = GetProfileNames(content);
        texts = GetTextFromProfile(content);
        for (var i = 0; i < names.Count; i++)
        {
            namesAndTexts.Add(string.Format(@"{0} {1}", names[i], texts[i]));

        }

        if (InvokeRequired)
        {
            result = tempNamesAndTexts.SequenceEqual(namesAndTexts);
            if (result == false)
            {
                var lastText = namesAndTexts.Last();
                Invoke(lastText);
                Log(lastText);
            }
        }

        tempNamesAndTexts = new List<string>();
        for (var i = 0; i < names.Count; i++)
        {

            tempNamesAndTexts.Add(string.Format(@"{0} {1}", names[i], texts[i]));
        }

        reader.Close();
        response.Close();
        Thread.Sleep(1000);
    }
}