4

I am refactoring some code an have a question that i could use a few comments on.

The original code download a file to a stream. Then it writes the stream to a File in a temp directory before using File.Copy to overwrite an existing file in the production directory.

Are there any benefits from writing it to the temp dir first and using the File.Copy in contrast to just writing the stream to the production directory right away?

One reason could be that the File.Copy is faster then writing a stream, and reducing the chance that someone is reading the file while its being written. But can that even happen? What else should I have in mind. I am considering factoring out the temp directory.

MemoryStream stream = new MemoryStream();
....Download and valiate stream....
using (Stream sourceFileStream = stream)
{
    using (FileStream targetFileStream = new FileStream(tempPath, FileMode.CreateNew))
    {
        const int bufferSize = 8192;
        byte[] buffer = new byte[bufferSize];
        while (true)
        {
              int read = sourceFileStream.Read(buffer, 0, bufferSize);
              targetFileStream.Write(buffer, 0, read);

              if (read == 0)
                  break;
        }
    }

}
File.Copy(tempPath, destination, true);

in contrast to just writing the stream to destination.

This is just the code I had, i would properly use something like sourceFileStream.CopyToAsync(TargetFileStream);

Poul K. Sørensen
  • 16,950
  • 21
  • 126
  • 283
  • Does the original file get deleted? In that case you have to be absolutely positively sure that the new file went where it was supposed to. I've seen the exact system you describe used quite a bit in automated file uploaders. However, File.Copy is a built in function, so it's probably wicked robust, meaning it throws an exception if ANYTHING goes wrong. – Jasmine May 02 '13 at 22:43
  • Also, please post the codez - there are some things that the Stream could be doing that aren't obvious. File.Copy makes an exact copy according to OS rules - streaming might not. – Jasmine May 02 '13 at 22:45
  • Will post a minimal solution in a few – Poul K. Sørensen May 02 '13 at 22:48
  • When using streams, you can choose whether to allow others to Read or Write while you use it. – SimpleVar May 02 '13 at 22:51

3 Answers3

6

well, think about what happens when you start the download and override the existing file and then for some reason the download gets aborted, you'd be left with an broken file. however, downloading it first in another location and copying it to the target directory factors that problem out.

EDIT: okay, seeing the code now. if the file is already in the MemoryStream there's really no reason to write the file to a temp location and copy it over. you could just just do File.WriteAllBytes(destination,stream.ToArray());

shriek
  • 5,157
  • 2
  • 36
  • 42
  • +1 - Also, the user doesn't necessarily have to click an abort button, they could be on a laptop where the battery dies during save – Sayse May 02 '13 at 22:30
  • I have downloaded the file to a stream an the download is completed. Question is in relation to if it makes a difference to write the stream(MemoryStream) to a temp file before copying it or not. The stream concent is validated with MD5 also. – Poul K. Sørensen May 02 '13 at 22:33
  • @s093294 That's his point - if you save the stream straight to the permanent location the download could be cut off for any number of reasons. That means you've lost the new file and you've also lost the original that was being overwritten. – Dave Zych May 02 '13 at 22:34
  • You are not getting the point, the download is not cutoff. When the download completes, the content is copye to a MemoryStream. The content is validated by MD5 Hash. – Poul K. Sørensen May 02 '13 at 22:38
  • s093294, *you* are not getting the point. You have to design your code to handle all possible future problems, not just what is happening today. The fact that your file finished properly today is meaningless. – Jasmine May 02 '13 at 22:40
  • ohhh, boy. Forgive me but, did you guys understand the question. Look apart from the download process. its not part of the question. The downloaded content is validated before it is copied to a MemoryStream. If the download fails, the operation is aborted. If it succeed it moves to the copy part. So unless you are referring to the downloading part as copying a MemoryStream to a FileStream(CLR TYPES), you have not understood the question. – Poul K. Sørensen May 02 '13 at 22:46
  • Wow. Just forget the fact that is downloads it in the first place. It is already DOWNLOADED, in past tense, and now he wants to save to file FROM MEMORY. And no, there is no difference. Just overwrite the file. – SimpleVar May 02 '13 at 22:52
1

File.Copy simply encapsulates the usage of streams, etc. No difference at all.

SimpleVar
  • 14,044
  • 4
  • 38
  • 60
1

It is a better practice to assemble the file stream of bytes into an isolated location and only after it is assembled to copy it to production area for the following reasons:

  1. Assume a power shortage during the assemble phase, when it is in isolated folder such as 'temp' you just end up with partial file which you can recheck and ignore later on.. however if you directly assemble the file on production and a power shortage occur - next time you turn on your application you will need to check integrity of all files which are not 'static' to your app.

  2. If the file is being used on production and 'being-assembled' new file is long - you end up having your user wait for the process of assembling to complete, However when the process of assembling the buffers is as in your example - simply copying the ready file will inflict shorter waiting period for your user.

  3. Assume the disk space is full... same problem as in #1.

  4. Assume Another process in your application has a memory leak and the application is crushed out before completing the assemble process.

  5. Assume [fill in your disaster case]...

So, yes. it is better practice to do as in your example, but the real question is how important is this file to your application? is it just another data file like a 'save-game' file or is it something that can crush your application if invalid?

G.Y
  • 6,042
  • 2
  • 37
  • 54
  • Valid points, but not the real question. We are not talking about assembling the file stream. It was already assembled. – Poul K. Sørensen May 02 '13 at 23:41
  • 1
    @s093294 I think you should consider your thoughts :) your 'karma' is broadcasting that you're upset to inherit someone else code.. I think we've all been there at least once cursing the guy who wrote what we need to sustain now.. but don't let this shade your academic point of view regarding what is better than what. you didn't ask to get an answer, you came to get confirmation the other guy is wrong :) forgive me for being honest - it just means I really sympathize with what you're going through ;) – G.Y May 03 '13 at 01:50
  • Not the case. I am not upset with the current code. I just did it another way and I am making sure that my way also works. – Poul K. Sørensen May 03 '13 at 08:48