Network Communication Adapter

Question

I'm creating a internet connection utility and below is the code I have used.
Is there any better way of check for proxy and internet connection etc?

        /// <summary>
    /// Check the Internet connection using the url passed into it
    /// Network Adapter Check - TRUE
    /// Proxy Settings Check - TRUE
    /// Speed Check - Depends on Parameter speedCheck
    /// Internet Connection Check - TRUE
    /// </summary>
    /// <param name="url">Url to validate connection</param>
    /// <param name="speedCheck">Validate Speed of Interenet Connection </param>
    /// <returns>Connection/No Connection</returns>        
    public static bool CheckInternet(string url, bool speedCheck)
    {
        speedCheckRequired = speedCheck;
        setProgressBar("Internet Connection", "Validation", 4);
        bool present = validateConnection(url);
        closeProgressBar();

        return present;
    }

 /// <summary>
    /// Check for proxy setting and if pressent gather proxy details
    /// </summary>
    /// <param name="web">WebClient Obeject</param>
    /// <returns>Setting Found/Not Found</returns>
    public static bool IsProxyRequired(string testUrl)
    {
        try
        {
            progressBar.SetStatus("Quering for proxy settings");                
            HttpWebRequest myWebRequest = (HttpWebRequest)WebRequest.Create(testUrl);                
            IWebProxy proxy = myWebRequest.Proxy;
            var settings = proxy.GetProxy(myWebRequest.RequestUri);                
            if (!settings.OriginalString.Equals(testUrl))
            {
                Utilities.DebugSolution.SetMessage("Utilities.NetworkCommunication.IsProxyRequired","Proxy setting found");
                Utilities.DebugSolution.SetMessage("Utilities.NetworkCommunication.IsProxyRequired", String.Format("Url: {0} Port: {1}",settings.AbsoluteUri,settings.Port));
                proxySettings = proxy;
                return true;
            }
        }
        catch (Exception e)
        {
            Console.WriteLine("Utilities.NetworkCommunication.IsProxyRequired Exception: " + e.Message.ToString());
            Utilities.DebugSolution.SetException("Utilies.NetworkCommunication.IsProxyRequired", e);
        }


        return false;
    }
    /// <summary>
    /// Make sure there is a network connection on the device
    /// </summary>
    /// <param name="status"></param>
    /// <returns>Connected/Not Connected</returns>
    public static bool IsNetorkAdapterConnected(ref string status)
    {
        bool result = false;
        status = "Network adapter not connected";
        try
        {
            progressBar.SetStatus("Quering network connections");  
            if (System.Net.NetworkInformation.NetworkInterface.GetIsNetworkAvailable())
            {
                result = true;
                status = String.Empty;
            }

        }catch (Exception e)
        {
            Console.WriteLine("Utilities.NetworkCommunication.IsNetorkAdapterConnected Exception: " + e.Message.ToString());
            Utilities.DebugSolution.SetException("Utilies.NetworkCommunication.IsNetorkAdapterConnected", e);
        }
        return result;
    }



    private static bool validateConnection(string url)
    {
        string status = String.Empty;
        if (IsNetorkAdapterConnected(ref status))
        {


            try
            {
                using (var client = new WebClient())
                {
                    if (IsProxyRequired(url))
                        client.Proxy = proxySettings;

                    try
                    {
                        progressBar.SetStatus("Checking internet connection");           
                        using (var stream = client.OpenRead(url))
                            return CheckSpeed(String.Empty);

                    }catch
                    {
                        ///This Catch says there is no internet connection available.
                        ///So no login\debug set here!
                    }

                }
            }
            catch (Exception e)
            {
                Console.WriteLine("Utilities.NetworkCommunication.validateConnection Exception: " + e.Message.ToString());
                Utilities.DebugSolution.SetException("Utilies.NetworkCommunication.validateConnection", e);
            }

        }//END if (IsNetorkAdapterConnected(ref status))

        return false;
    }

Answer 1

/// <returns>Connection/No Connection</returns>

When a method returns bool, you should make it very clear what do the true and false values mean. The intention is pretty clear here, but if it was e.g.:

/// <returns>Ascending/Descending</returns>

that would confuse me a lot. Documentation shouldn't make you guess.

---

setProgressBar("Internet Connection", "Validation", 4);
bool present = validateConnection(url);
closeProgressBar();

What happens when validateConnection throws an exception that is handled higher up the call stack? I think it will leave the progress bar open, but stuck.

One way to fix this would be to make setProgressBar return IDisposable and then use using. This has the added advantage that you don't need the local present anymore:

using (setProgressBar("Internet Connection", "Validation", 4))
{
    return validateConnection(url);
}

Also, if you have two methods that logically form a pair, their names should form a pair too. Consider names like open/close, enable/disable, or maybe set/reset.

---

/// <param name="web">WebClient Obeject</param>

This method doesn't have any such parameter. You should always make sure that your documentation and your code match.

Also, I believe that no documentation is better than useless documentation. Saying that the parameter of type WebClient is an "WebClient Obeject" doesn't tell the user of your code anything new.

---

/// <param name="status"></param>

Similar to above: if you don't want to document the parameter, don't, remove this line completely.

---

public static bool IsNetorkAdapterConnected(ref string status)

This method doesn't read status, so it should be an out parameter, instead of ref. That way, the signature more clearly documents what the method does and the callers of this method also won't have to assign some default value to their status local variable.

---

bool result = false;
status = "Network adapter not connected";
try
{
    progressBar.SetStatus("Quering network connections");  
    if (System.Net.NetworkInformation.NetworkInterface.GetIsNetworkAvailable())
    {
        result = true;
        status = String.Empty;
    }

}catch (Exception e)
{
    Console.WriteLine("Utilities.NetworkCommunication.IsNetorkAdapterConnected Exception: " + e.Message.ToString());
    Utilities.DebugSolution.SetException("Utilies.NetworkCommunication.IsNetorkAdapterConnected", e);
}
return result;

Always having only one return in a method is generally a good practice in C, but it's not needed in C# and I think that using multiple returns would make this code simpler and clearer:

try
{
    progressBar.SetStatus("Quering network connections");  
    if (System.Net.NetworkInformation.NetworkInterface.GetIsNetworkAvailable())
    {
        status = String.Empty;
        return true;
    }

} catch (Exception e)
{
    Console.WriteLine("Utilities.NetworkCommunication.IsNetorkAdapterConnected Exception: " + e.Message.ToString());
    Utilities.DebugSolution.SetException("Utilies.NetworkCommunication.IsNetorkAdapterConnected", e);
}

status = "Network adapter not connected";
return false;

---

System.Net.NetworkInformation.NetworkInterface.GetIsNetworkAvailable()

You should use using System.Net.NetworkInformation for this.

---

if (IsNetorkAdapterConnected(ref status))
{


    try
    {

What's with the empty lines here? Empty lines are useful for separating parts of methods, but I don't see any reason for them here.

---

using (var stream = client.OpenRead(url))
    return CheckSpeed(String.Empty);

This is confusing. Why do you have the stream variable that's never used, and why do you close the stream only after CheckSpeed returns?

Also, have you considered using the HEAD method, which doesn't return any data at all? I think that would require you to use WebRequest instead of WebClient.