File Copy Program Doesn't Properly Copy File

Question

Hello

I've been working on terminal-like application to get better at programming in c#, just something to help me learn. I've decided to add a feature that will copy a file exactly as it is, to a new file... It seems to work almost perfect. When opened in Notepad++ the file are only a few lines apart in length, and very, very, close to the same as far as actual file size goes. However, the duplicated copy of the file never runs. It says the file is corrupt. I have a feeling it's within the methods for reading and rewriting binary to files that I created. The code is as follows, thank for the help. Sorry for the spaghetti code too, I get a bit sloppy when I'm messing around with new ideas.

Class that handles the file copying/writing

using System;
using System.IO;
//using System.Collections.Generic;
namespace ConsoleFileExplorer
{
class FileTransfer
{
    private BinaryWriter writer;
    private BinaryReader reader;
    private FileStream fsc;    // file to be duplicated
    private FileStream fsn;    // new location of file 

    int[] fileData;
    private string _file;

    public FileTransfer(String file)
    {
        _file = file;
        fsc = new FileStream(file, FileMode.Open);
        reader = new BinaryReader(fsc);
    }

    // Reads all the original files data to an array of bytes 
    public byte[] ReadAllDataToArray() 
    {
        byte[] bytes = reader.ReadBytes((int)fsc.Length); // reading bytes from the original file
        return bytes;
    }

    // writes the array of original byte data to a new file
    public void WriteDataFromArray(byte[] fileData, string path) // got a feeling this is the problem :p
    {
        fsn = new FileStream(path, FileMode.Create);
        writer = new BinaryWriter(fsn);
        int i = 0;
        while(i < fileData.Length)
        {
            writer.Write(fileData[i]);
            i++;
      }
    }
  }
}

Code that interacts with this class .

(Sleep(5000) is because I was expecting an error on first attempt...

                    case '3':
                    Console.Write("Enter source file: ");
                    string sourceFile = Console.ReadLine();
                    if (sourceFile == "")
                    {
                        Console.Clear();
                        Console.ForegroundColor = ConsoleColor.DarkRed;
                        Console.Error.WriteLine("Must input a proper file path.\n");
                        Console.ForegroundColor = ConsoleColor.White;
                        Menu();
                    } else {
                        Console.WriteLine("Copying Data"); System.Threading.Thread.Sleep(5000);
                        FileTransfer trans = new FileTransfer(sourceFile);

                        //copying the original files data
                        byte[] data = trans.ReadAllDataToArray();

                        Console.Write("Enter Location to store data: ");
                        string newPath = Console.ReadLine();

                        // Just for me to make sure it doesnt exit if i forget
                        if(newPath == "")
                        {
                            Console.Clear();
                            Console.ForegroundColor = ConsoleColor.DarkRed;
                            Console.Error.WriteLine("Cannot have empty path.");
                            Console.ForegroundColor = ConsoleColor.White;
                            Menu();
                        } else
                        {
                            Console.WriteLine("Writing data to file"); System.Threading.Thread.Sleep(5000);
                            trans.WriteDataFromArray(data, newPath);
                            Console.WriteLine("File stored.");
                            Console.ReadLine();
                            Console.Clear();
                            Menu();
                        }
                    }
                break;

File compared to new file right-click -> open in new tab is probably a good idea

Original File

New File

Answer 1

There is an File.Copy() Method in C#, you can see it here https://msdn.microsoft.com/ru-ru/library/c6cfw35a(v=vs.110).aspx

If you want to realize it by yourself, try to place a breakpoint inside your methods and use a debug. It is like a story about fisher and god, who gived a rod to fisher - to got a fish, not the exactly fish.

Also, look at you int[] fileData and byte[] fileData inside last method, maybe this is problem.

Answer 2

You're not properly disposing the file streams and the binary writer. Both tend to buffer data (which is a good thing, especially when you're writing one byte at a time). Use using, and your problem should disappear. Unless somebody is editing the file while you're reading it, of course.

BinaryReader and BinaryWriter do not just write "raw data". They also add metadata as needed - they're designed for serialization and deserialization, rather than reading and writing bytes. Now, in the particular case of using ReadBytes and Write(byte[]) in particular, those are really just raw bytes; but there's not much point to use these classes just for that. Reading and writing bytes is the thing every Stream gives you - and that includes FileStreams. There's no reason to use BinaryReader/BinaryWriter here whatsover - the file streams give you everything you need.

A better approach would be to simply use

using (var fsn = ...)
{
  fsn.Write(fileData, 0, fileData.Length);
}

or even just

File.WriteAllBytes(fileName, fileData);

Maybe you're thinking that writing a byte at a time is closer to "the metal", but that simply isn't the case. At no point during this does the CPU pass a byte at a time to the hard drive. Instead, the hard drive copies data directly from RAM, with no intervention from the CPU. And most hard drives still can't write (or read) arbitrary amounts of data from the physical media - instead, you're reading and writing whole sectors. If the system really did write a byte at a time, you'd just keep rewriting the same sector over and over again, just to write one more byte.

An even better approach would be to use the fact that you've got file streams open, and stream the files from source to destination rather than first reading everything into memory, and then writing it back to disk.