writing to text file does not alway work/save

Question

i have this code that compares two text files and write the difference to a log file but for some reason the log.txt file is some times blank even when is test with some lines starting with a * these are not always written either do I have to save the text file when finished writing although this does not explain why sometimes it works any help would be great

private void compare()
{
  string FilePath = @"c:\snapshot\1.txt";
  string Filepath2 = @"c:\snapshot\2.txt";
  int counter = 0;
  string line;
  string line2;

  var dir = "c:\\snapshot\\log.txt";

  using (FileStream fs = File.Create(dir))
  {
    fs.Dispose();
  }

  StreamWriter dest = new StreamWriter(dir);

  if (File.Exists(FilePath) & File.Exists(Filepath2))
  {
    // Read the file and display it line by line.
    using (var file = File.OpenText(FilePath))
    using (var file2 = File.OpenText(Filepath2))
    {
      while (((line = file.ReadLine()) != null & (line2 = file2.ReadLine()) != null))
      {
        if (line.Contains("*"))
        {
          dest.WriteLine(line2);
        }
        else if (!line.Contains(line2))
        {
          dest.WriteLine(line2);
        }
        counter++;
      }
    }
  } 
  dest.Close();
}

At a cursory glance, it looks to me like your lines can get out of sync since you're reading two files with varying data. If the files aren't large, you might want to load them into lists and to a more formal comparison. — IamIC, Aug 27 '12 at 13:03
There's some mess in your code, if you're using "using", you don't need to call dispose, why aren't you using "using" for StreamWriter, why using FileStream and doing nothing with it... — Giedrius, Aug 27 '12 at 13:03
Btw, the whole point of the `using` statement, it that it will call Dispose for you, so calling Dispose in a `using` block, is, at best, not needed. — SWeko, Aug 27 '12 at 13:04
Use `&&`, not `&`. The single `&` is bitwise arithmetic that may end up doing what you want, but generally here you mean `&&`. — James Cronen, Aug 27 '12 at 13:07
I am new to c# and programming just teaching myself and getting tips for the forum so something else to add to the list dont need to use dispose when using "using" — Kevin Babb, Aug 27 '12 at 13:08
If anyone told you to do this like that, slap them! Saying that my first guess would be to call dest.Flush(), before dest.Close(). — Tony Hopkinson, Aug 27 '12 at 13:13

score 1 · Answer 1 · answered Aug 27 '12 at 13:08

1

Everything left in the buffer should be written out once you hit the close statement on your StreamReader. If you are missing stuff then it might be that you aren't reaching that line for some reason (i.e. you crash). Also, if you are trying to look at the file while it's being written (i.e. while the program is still running), you won't necessarily see everything (since it hasn't closed).

Generally, it's better to use a using statement with the StreamReader. That should ensure that it always gets closed.

answered Aug 27 '12 at 13:08

Matt Burland

44,552
18
99
171

whats the best way to load both text files to a list then compare the lists? – Kevin Babb Aug 27 '12 at 13:16
NB if you already have a using round the input to StreamReader, e.g. a Filestream, then putting one on StreamReader as well will give you an already disposed error when the FileStream instance goes out of scope. – Tony Hopkinson Aug 27 '12 at 14:06

score 0 · Answer 2 · answered Aug 27 '12 at 13:29

Not sure if I'm understanding your comparing logic right, but as long as I separated comparing from whole code you can adjust it to your own needs:

    public static void WriteDifferences(string sourcePath, string destinationPath, string differencesPath)
    {
        var sourceLines = File.ReadAllLines(sourcePath).ToList();
        var destinationLines = File.ReadAllLines(destinationPath).ToList();            

        // make lists equal size
        if (sourceLines.Count > destinationLines.Count)
        {
            destinationLines.AddRange(Enumerable.Range(0, sourceLines.Count - destinationLines.Count).Select(x => (string)null));
        } 
        else
        {
            sourceLines.AddRange(Enumerable.Range(0, destinationLines.Count - sourceLines.Count).Select(x => (string)null));
        }

        var differences = sourceLines.Zip(destinationLines, (source, destination) => Compare(source, destination));

        File.WriteAllLines(differencesPath, differences.Where(x => x != null));
    }

    private static string Compare(string source, string destination)
    {
        return !source.Contains(destination) || source.Contains("*") ? destination : null;
    }

Not sure I'm mad keen on your assumption that's there's enough memory swanning about doing nothing to hold both files and the differences... — Tony Hopkinson, Aug 27 '12 at 14:02
It depends. If I'm sure, that files never exceed 1mb, I would prefer easy maintainable code, if not - sure, there's no problem by writing line by line reading and comparing, which probably would be slower than current approach(not sure how io caching would work out for reading line by line). — Giedrius, Aug 27 '12 at 14:13
If I knew the files were small, I have no problem with it. Seeing as we don't know that, just thought I'd mention the issue, for those who don't like intermittent OOMs is their code :( — Tony Hopkinson, Aug 27 '12 at 14:19

score 0 · Accepted Answer · answered Aug 27 '12 at 14:14

private void compare()
{
  string FileName1 = @"c:\snapshot\1.txt";
  string FileName2 = @"c:\snapshot\2.txt";
  string FileNameOutput = @"c:\snapshot\log.txt"; //dir ???
  int counter = 0; // um what's this for you aren't using it.

  using (FileStream fso = new FileStream(FileNameOutput, FileMode.Create, FileAccess.Write))
  {
    TextWriter dest = new StreamWriter(fso);
    using(FileStream fs1 = new FileStream(FileName1, FileMode.Open, FileAccess.Read))
    {
      using (FileStream fs2 = new FileStream(FileName2, FileMode.Open, FileAccess.Read))
      {
        TextReader firstFile = new StreamReader(fs1);
        TextReader secondFile = new StreamReader(fs2);
        while (((line1 = firstFile.ReadLine()) != null & (line2 = secondFile.ReadLine()) != null))
        {
          if ((line1.Contains("*") || (!line1.Contains(line2)))
          {
            dest.Write(line2); // Writeline would give you an extra line?
          }
          counter++; // 
        }
      }
    }
  fso.Flush();
}

I commend the overloads of FileStream to you. Do it the way I have and the code will crash at the point you instance the stream if the user running it doesn't have all the required permissions. It's a nice way of showing what you intend, and what you don't.

PS You do know contains is case and culture sensitive?

I think you don't need manual flush: http://stackoverflow.com/questions/7710661/do-you-need-to-call-flush-on-a-stream-if-you-are-using-using-statement — Giedrius, Aug 27 '12 at 14:15
Hmmm, I know it says you shouldn't but I've seen it fix issues several times in a sort of belt and braces catch all approach. I don't trust that it will happen so I call it in self defence now. — Tony Hopkinson, Aug 27 '12 at 14:24

writing to text file does not alway work/save

3 Answers3