0

I am trying to read/write files using ReadBytemethod. code is working but I have noticed that they are not available after process.I cant open them.I images not displaying.what am i doing wrong again and again.

 if (openFileDialog1.ShowDialog() == DialogResult.OK) {
     if (saveFileDialog1.ShowDialog() == DialogResult.OK) {
          FileStream fsRead = 
              new FileStream(openFileDialog1.FileName, FileMode.Open);
          FileStream fswrite = 
              new FileStream(saveFileDialog1.FileName, FileMode.Create);

         if (fsRead.Position != fsRead.Length) {
             byte b = (byte)fsRead.ReadByte();
             fswrite.WriteByte(b);
         }      
     }
}
mihai
  • 4,592
  • 3
  • 29
  • 42
user3733078
  • 249
  • 2
  • 11
  • If you want to copy a file, there are much better ways to do it than copying byte-by-byte, which has very low performance. 'System.IO.File.Copy' is the first one that comes to mind. – qbik Jun 22 '14 at 07:47

2 Answers2

5

You're only reading a single byte - I suspect you meant to write a while loop instead of an if statement:

while (fsRead.Position != fsRead.Length) {
   byte b = (byte)fsRead.ReadByte();
   fswrite.WriteByte(b);
}

However, that's still not very efficient. Typically it's better to read and write chunks at a time, using "I can't read any more" to indicate the end of the file:

byte[] buffer = new byte[8192];
int bytesRead;
while ((bytesRead = fsRead.Read(buffer, 0, buffer.Length)) > 0) {
    fswrite.Write(buffer, 0, bytesRead);
}

However, you don't really need to do this yourself, as you can use Stream.CopyTo to do it for you:

fsRead.CopyTo(fswrite);

Note that you should also use using statements for your streams, to close them automatically at the end of the statement. I'd also use File.OpenWrite and File.OpenRead rather than calling the FileStream constructor, and just use a Stream variable:

using (Stream read = File.OpenRead(openFileDialog1.FileName),
             write = File.OpenWrite(saveFileDialog1.FileName))
{
    read.CopyTo(write);
}

Or just use File.Copy of course!

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • using (Stream read = File.OpenRead(openFileDialog1.FileName), write = File.OpenRead(saveFileDialog1.FileName)). Didn't know using supported that syntax. – Rezo Megrelidze Jun 22 '14 at 07:52
  • 1
    @rezomegreldize: It's just like declaring two variables of the same type normally. It *only* works when they're the same type though, so you can't use this approach to open (say) a `SqlConnection` and a `SqlCommand`. – Jon Skeet Jun 22 '14 at 07:58
  • thank you for your help and detailed explanation Jon.But I wonder is that any difference [8192] and[1024] while reading bytes. – user3733078 Jun 22 '14 at 08:00
  • @user3733078: With 8192 it will take a bit more memory, but probably use fewer file operations. Both will work, and you could go up to 32K with no issues. I'd stop there on the grounds of not wanting to get into Large Object Heap territory, but it's up to you. – Jon Skeet Jun 22 '14 at 08:07
  • The various ways to handle nested using statements are discussed at on SO: [Nested using statements in C#](http://stackoverflow.com/q/1329739/18192). – Brian Jun 23 '14 at 13:37
0

You need to close files after using, they are locked until that.

Best practice is to use using (var fs = new FileStream(...) { ... } construct - in that case, file streams and underlying files will be closed after using scope is finished.

So it should be something like this:

if (openFileDialog1.ShowDialog() == DialogResult.OK) {
    if (saveFileDialog1.ShowDialog() == DialogResult.OK) {
        using (FileStream fsRead = new FileStream(openFileDialog1.FileName, FileMode.Open))
        using (FileStream fswrite = new FileStream(saveFileDialog1.FileName, FileMode.Create)) {
            // your logic here
            if (fsRead.Position != fsRead.Length) {
               byte b = (byte)fsRead.ReadByte();
               fswrite.WriteByte(b);
            }
        }
    }
Lanorkin
  • 7,310
  • 2
  • 42
  • 60