Skip to main content
added 283 characters in body
Source Link
svick
  • 24.5k
  • 4
  • 53
  • 89

First, like ANeves said, the only thing your catch does is that it resets the exception's stack trace, which is not what you want to do. Just remove the try/catch completely, it serves no purpose here.

Second, I don't see any reason to use WebRequest here, using WebClient is much simpler.

Also, you might want to have better parameter names: fileloc doesn't say what exactly it is, and doesn't use the usual capitalization. And loc is unnecessarily abbreviated (you're not trying to write a Twitter post where every character counts, readability is more important) and too ambiguous.

public void DownloadFile(string url, string savePath)
{
    var client = new WebClient();
    client.Credentials = new NetworkCredential(Username, Password);
    client.DownloadFile(url, savePath);
}

This method could be rewritten as one expression, but I probably wouldn't do that:

public void DownloadFile(string url, string savePath)
{
    new WebClient { Credentials = new NetworkCredential(Username, Password) }
        .DownloadFile(url, savePath);
}

First, like ANeves said, the only thing your catch does is that it resets the exception's stack trace, which is not what you want to do. Just remove the try/catch completely, it serves no purpose here.

Second, I don't see any reason to use WebRequest here, using WebClient is much simpler.

Also, you might want to have better parameter names: fileloc doesn't say what exactly it is, and doesn't use the usual capitalization. And loc is unnecessarily abbreviated (you're not trying to write a Twitter post where every character counts, readability is more important) and too ambiguous.

public void DownloadFile(string url, string savePath)
{
    var client = new WebClient();
    client.Credentials = new NetworkCredential(Username, Password);
    client.DownloadFile(url, savePath);
}

First, like ANeves said, the only thing your catch does is that it resets the exception's stack trace, which is not what you want to do. Just remove the try/catch completely, it serves no purpose here.

Second, I don't see any reason to use WebRequest here, using WebClient is much simpler.

Also, you might want to have better parameter names: fileloc doesn't say what exactly it is, and doesn't use the usual capitalization. And loc is unnecessarily abbreviated (you're not trying to write a Twitter post where every character counts, readability is more important) and too ambiguous.

public void DownloadFile(string url, string savePath)
{
    var client = new WebClient();
    client.Credentials = new NetworkCredential(Username, Password);
    client.DownloadFile(url, savePath);
}

This method could be rewritten as one expression, but I probably wouldn't do that:

public void DownloadFile(string url, string savePath)
{
    new WebClient { Credentials = new NetworkCredential(Username, Password) }
        .DownloadFile(url, savePath);
}
Source Link
svick
  • 24.5k
  • 4
  • 53
  • 89

First, like ANeves said, the only thing your catch does is that it resets the exception's stack trace, which is not what you want to do. Just remove the try/catch completely, it serves no purpose here.

Second, I don't see any reason to use WebRequest here, using WebClient is much simpler.

Also, you might want to have better parameter names: fileloc doesn't say what exactly it is, and doesn't use the usual capitalization. And loc is unnecessarily abbreviated (you're not trying to write a Twitter post where every character counts, readability is more important) and too ambiguous.

public void DownloadFile(string url, string savePath)
{
    var client = new WebClient();
    client.Credentials = new NetworkCredential(Username, Password);
    client.DownloadFile(url, savePath);
}