Checking URL validity This is sort of a follow up to a previous question (Do I have any SQL leaks?).

I have changed some things around and am working on a solution where I use Active Directory to gather information about the user rather than use the user table from the application that is run on these App Servers (the thing I am trying to keep track of).

I think that this method might be better utilized if it were written differently:

public bool isValid(string url)
{
    HttpWebRequest urlReq;
    HttpWebResponse urlRes;
    try
    {
        urlReq = (HttpWebRequest)WebRequest.Create(url);
        urlReq.Method = "HEAD";
        urlReq.Timeout = 200000;
        urlRes = (HttpWebResponse)urlReq.GetResponse();
        urlRes.Close();
        return true;
    }
    catch (Exception ex)
    {
        //Url not valid
        strErrorMSG = "Exception From isValid Method. Exception to follow: " + ex.ToString();
        return false;
    }
}


And this is how I am currently using it:

if (isValid(strInput))
{
    try
    {
        XmlReader xmlReader = XmlReader.Create(strInput);
        using (xmlReader)
        {
            while (xmlReader.Read())
            {
                if (xmlReader.NodeType == XmlNodeType.Text)
                {
                    strServer = xmlReader.Value.ToString();
                    strServer = strServer.Replace("\r", "");
                    strServer = strServer.Replace("\n", "");
                    strServer = strServer.Replace(" ", "");
                }
            }
            xmlReader.Close();
        }
    }
    catch (Exception e)
    {
        strServer = "Error";
        strErrorMSG = "Error with the xmlReader Exception as follows: " + e.ToString();
    }
}
else
{
    strServer = "XML-Missing";
}


I take this information and then I store it in an SQL table where I pull the information from it to populate a data grid on a website so I can see how many users are on each app server (using the App Server indirectly by running the application).

The variables strServer and strErrorMSG (are horrible names I know, I may refactor that as well while I am working on the version) get inserted into an SQL table so that I can see that there is an error with the Windows Service and have an error message that I can grab.

One of the things on my to-do list is to add a function that will email me with errors should the SQL Server not be accessible from the client machine.

I only use 1 SQL table at the moment for all the information that is gathered with this Windows service, another reason why I am working on the e-mail functionality.

Note: This is code from a Windows service that runs on almost 1000 computers.

How does my boolean method look, and can I tweak this somehow to make it work with the XmlReader more efficiently (or even at all)?



I created this Windows service to track which AppServer a user's application is connected to via a load balancer.

I install an XML on each AppServer with their own unique "server" node(which is pretty much the only thing there), the windows service needs to query that XML and return the value in that node giving it the server that the application is connected to. After that the Windows service sends the information to a SQL table so that it can be viewed via a website.

The reason for the weird logging is because this service is running on somewhere around 1000 computers in the state, and I am already sending information to the SQL Table, where the information is being displayed on the website(what we monitor frequently) so this is the fastest way at the moment for seeing the error messages.  I am looking into a way where I can have the exceptions E-mailed to me instead of only sending to the Table so that I don't have to stare at the site all the time.

I agree that I don't like the isValid method and that it creates an extra request.  What I am looking for is a way to merge it with the XmlReader and some of you have given me really good ideas how to accomplish this and I appreciate it greatly.
It can be tough, but pretend you're not the developer who wrote this code and try to understand what it does by reading through it. At a quick glance some questions you might ask could be:


What does isValid actually check the validity of?
What is strInput? The line isValid(strInput) just tells me that someone has deemed some input string valid for doing something. Not very useful, right?
When there is an exception in the XMLReader portion, what does it mean to just set the exception text to a random set of instance variables and continue on like nothing happened?


Some quick fixes might be to just use better variable/method names such as: isResolvableUrl(url) and inputXmlUrl.

On to your actual code...it's a very bad practice to use try/catch blocks for flow control. Exceptions should be exceptional (see: When to throw an exception)! Right now, you're just brushing exceptions under the carpet, which makes the rest of your application unable to properly handle them.

Think about the actual flow of your application in an ideal case. First, you make a request to verify if the file is there, which has network cost. Then, you use the same url to create an XmlReader which has to read from there again! Since you're catching all exceptions, you're better off not doing a check to see if it'll fail and just letting it fail without doing the check in the first place.

If each of your methods had a single identifiable functionality, you'll find you don't need all the try/catch blocks:

public XmlReader GetXmlFromRemoteLocation(string xmlLocation)
{ }

public void ReplaceWhitespace(XmlReader xml)
{ }

public void WhateverYourMainMethodIs()
{
    // Let it throw Exceptions that can be caught!
}

// Alternatively (if you must):
public bool TryWhateverYourMainMethodIs()
{
    try { }
    catch ( ExpectedExceptionTypes )
    { }

    return success;
}