Is this FTP download code useful?

Question

I have a Windows-service and this service will connect to an ftp server and do download, upload, and rename operations.

My service runs every 30 seconds, so the ftp server will be busy during these operations.

My question is: is my code useful for this job? Because my download/upload/rename functions open a webrequest each process? I use FtpWebRequest and FtpWebResponse

I write only the download functions, the other functions are similar.

Download function:

    public Result Download(string localPath,string remotePath,string fileName)
    {
        Result _result = new Result();
        FtpWebRequest requestFileDownload = null;
        FtpWebResponse responseFileDownload = null;

        try
        {

            //
            requestFileDownload =
                (FtpWebRequest)WebRequest.Create(remotePath + fileName);
            //
            requestFileDownload.Credentials =
                new NetworkCredential(this.Username, this.Password);
            //
            requestFileDownload.Method =
                WebRequestMethods.Ftp.DownloadFile;

            responseFileDownload =
                (FtpWebResponse)requestFileDownload.GetResponse();

            using (Stream responseStream = responseFileDownload.GetResponseStream())
            {
                FileStream writeStream = new FileStream(localPath + fileName, FileMode.Create);

                int Length = 2048;
                Byte[] buffer = new Byte[Length];

               int bytesRead;
               while((bytesRead = responseStream.Read(buffer, 0, Length)) > 0)
               {
                  writeStream.Write(buffer, 0, bytesRead);
               }

                writeStream.Close();

            }

            return _result;

        }
        catch (Exception x)
        {
            _result.Error = true;
            _result.ErrorMessage = x.StackTrace;
            _result.ErrorFileName = fileName;
            _result.ErrorFilePath = remotePath + fileName;

            return _result;
        }
        finally
        {
            requestFileDownload = null;
            responseFileDownload = null;

        }


    }

    public struct Result
    {
        public bool Error;
        public string ErrorMessage;
        public string ErrorFileName;
        public string ErrorFilePath;
        public List<string> Files;
    }

Answer 1

A few things:

1. I would supply all the variables that you have hard coded into the method i.e. localPath, filename, ftp server, networkcredentials. Otherwise how can you re-use this method for other operations?

2. I would probably avoid making the method static unless there was good reason to.

3. In your finally clause you don't check if responseStream or writeStream are null? What if they never got created before an exception was thrown?

4. I personally think unless there is a very valid reason you should always do something in an exception catch block, whether it be logging somewhere, re-throwing the exception or anything. Otherwise the calling method will have no idea that something went wrong.

5. What is _result? I don't see it being set and I don't see it being declared anywhere?

6. If Length is a constant, make it a constant. i.e. const int Length = 2048.

7. I might also consider implementing some sort of call back method so you can provide status updates on the progress on the download. Just a suggestion, not sure how you would implement it exactly.

I might consider writing the loop as such:

int bytesRead;
while((bytesRead = responseStream.Read(buffer, 0, Length)) > 0)
{
   writeStream.Write(buffer, 0, bytesRead);
}

As to your quesion. Is this code useful? In short. Yes, but only if you ever want to do only one thing with it and never want to re-use anything inside. Otherwise no.

That's just a few comments off the top of my head. I hope those are a good starting point.

Answer 2

If this process executes more than a single command in each interval, such as: upload something, then rename a file, then download something else - then it's not optimized.

I suggest you use some kind of a library for FTP session, in which you should execute all of the commands in a single session. This should be better in terms of performance, since you're already connected to the FTP server, and therefore you already passed the firewalls and the FTP authentication.

Update: so far I used several FTP libraries, some were free (long time ago, around .NET 1.1), some not (recently). I can't recommend a free library since I haven't recently used one. I suggest visiting free ftp library question (and answers) in stackoverflow for a discussion about the subject. However, I recently used the non free Rebex FTP library, and it's excellent.