FTP Download Function

Question

This is an function that downloads a single file from an FTP server and saves it to disk This is my first time doing anything with FTP, please let me know if i have missed anything.

public void DownloadFile(string fileloc, string saveLoc)
{
 try
 {
 // Get the object used to communicate with the server.
 FtpWebRequest request = (FtpWebRequest)WebRequest.Create(fileloc);
 request.Method = WebRequestMethods.Ftp.DownloadFile;
 request.Credentials = new NetworkCredential(Username, Password);
 using (var response = (FtpWebResponse)request.GetResponse())
 {
 using (var fileStream = File.Create(saveLoc))
 {
 response.GetResponseStream().CopyTo(fileStream);
 }
 }
 }
 catch (Exception EX)
 {
 throw EX; 
 }
}

Answer 1

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);
}

• Philippe Lavoie: Almost everything I have to say.

Answer 2

When you rethrow an exception using throw ex; you refresh its stacktrace. (Frame?) This means the exception will seem to have been initially raised at your throw ex; line, and loses its previous stacktrace.

You want to simply throw; - like this:

try {
 new Profiterole(); // BOOM!
} catch (Exception ex) {
 throw; // Preserves the stacktrace, showing that the Exception was raised in InitGlacing();
}

---

As suggested by @svick: better yet, don't have a try/catch at all; it serves no purpose here.

• svick: Better yet, don't have a try/catch at all. It serves mo purpose here.

Answer 3

It is always good to have a buffer parameter or at least overload the function to provide one. I think this justifies using the FtpWebRequest vs the WebClient as increasing your buffer can greatly increase your transfer speed and saturate all available network bandwidth. You will not see any improvement on small files but if you are downloading anything over 100mb is when you will start seeing a noticiceable difference.

Just overfload the CopyTo as

responseStream.CopyTo(fileStream, buffer)

And it will do all the work for you. Default is 4096 bytes (4kb). It will take some testing to find just the right buffer but I have found 32kb works well.