Writing to a stream losing data

[tshad]

I have a class that write text to a textfile (csv file).

The problem is that it drops off the last 10 lines or so.

I know all the data is getting written to the stream but the it is like the
last bit of the buffer is not getting written out. I close the writer in
reverse order from what I opened it with - so I don't think that is the
problem.

Anyone know what would cause that?


The last thing I do in my code is the .close().

Here is my class. Am I missing anything?

****************************************************
using System;
using System.IO;
using System.Data;
using System.Text;

namespace AutoUPS
{
public class CsvWriter
{
private StringBuilder stringOut = null;
private StreamWriter csvWriter = null;
private string fileName;
bool newLine = true;
FileStream fs = null;
StreamWriter csvFileWriter = null;

public CsvWriter(string fileName)
{
this.fileName = fileName;
fs = new FileStream(fileName, FileMode.Create,
FileAccess.ReadWrite);
csvFileWriter = new StreamWriter(fs);
}
~CsvWriter()
{
//csvWriter.Close();
}
public void Close()
{
if(csvWriter != null)
csvWriter.Close();
if (fs != null)
fs.Close();
}

public void WriteToString(string input)
{
if(stringOut == null)
stringOut = new StringBuilder();

WriteToString(input, true);
}
public void WriteToString(string input, bool quoteall)
{
if (!newLine)
stringOut.Append(",");

if (quoteall || input.IndexOfAny("\",\x0A\x0D".ToCharArray())

-1)

stringOut.Append("\"" + input.Replace("\"", "\"\"") + "\"");
else
stringOut.Append(input);
newLine = false;
}
public void WriteEOL()
{
stringOut.Append(Environment.NewLine);
newLine = true;
}

public void WriteToStream()
{
if (stringOut != null)
{
csvFileWriter.Write(stringOut.ToString());
stringOut = null;
}
}
public string GetText()
{
return stringOut.ToString();
}
}
}
*******************************************************

Thanks,

Tom

 

[tshad]

I also tried by only using the StreamWriter and got the same results:

public CsvWriter(string fileName)
{
this.fileName = fileName;
csvFileWriter = new StreamWriter(fileName);
}
~CsvWriter()
{
//csvWriter.Close();
}
public void Close()
{
if (csvWriter != null)
{
csvWriter.Flush();
csvWriter.Close();
}
}

Tom

 

[Family Tree Mike]

I also tried by only using the StreamWriter and got the same results:

public CsvWriter(string fileName)
{
this.fileName = fileName;
csvFileWriter = new StreamWriter(fileName);
}
~CsvWriter()
{
//csvWriter.Close();
}
public void Close()
{
if (csvWriter != null)
{
csvWriter.Flush();
csvWriter.Close();
}
}

Tom

You changed something key between posts. I was going to suggest calling
Flush() before closing. Are you sure you have flushed the output
before closing in your code where this fails?

 

[tshad]

Found it.

I was closing csvWrite and not csvFileWrite.

I am curios about whether I even need the FileStream object as I did in the
original post since the StreamWriter seems to work fine by itself?

Do I need it?

Thanks,

Tom

 

[Peter Duniho]

Found it.

I was closing csvWrite and not csvFileWrite.

I am curios about whether I even need the FileStream object as I did in the
original post since the StreamWriter seems to work fine by itself?

Do I need it?

No, you don't. A Stream wrapped by a TextWriter will be automatically
closed when the TextWriter is closed. Same thing for TextReader.

In the code example you posted, you should go ahead and delete the
finalizer completely. It's also not clear why you even have the
csvWriter variable, and it should be obvious the maintenance trap it
causes.

You also should move the initialization of "stringOut" into the
WriteToString(string, bool) overload.

It seems to me you also have an API problem, in that the code obviously
requires that the caller _not_ include an unquoted EOL character in the
passed-in string, but there's nothing in the code that prevents that.

To be consistent with the .NET API, you should also rename the GetText()
method to ToString(), making it an override like the
StringWriter.ToString() method.

Finally, frankly the whole business of having a single class that both
writes to a StringBuilder and to a FileStream is a really bad idea.
Classes should stay focused on a single task; if you want more complex
functionality, build it from simpler, task-specific classes.

In this particular example, it's trivial to remove the
FileStream-related functionality from the CsvWriter class and move it
elsewhere. That elsewhere would simply maintain a StreamWriter
instance, initialized as desired, and then call the TextWriter.Write()
method with the return value from the CsvWriter.GetText() method (or its
renamed incarnation, ToString() ).

In fact, IMHO your CsvWriter class should follow the .NET examples, and
always write to some TextWriter. When you want it to write to a string,
you can initialize it with a StringWriter, and then when you need to get
the text so far, you just get it directly from the StringWriter, rather
than the CsvWriter class. This also provides the flexibility of not
having to have an intermediate string buffer at all, or (if you want to
get really fancy) inserting some kind of Stream or TextWriter T
implementation so that the output goes to multiple destinations.

Alternatively, rename CsvWriter to CsvStringWriter to make it clear that
it always writes to a string. In this case, even better would be to
make it a sub-class of TextWriter.

Again, in any of those options, the goal is to keep the class special,
and allow more complex behaviors to be easily built by combining
existing classes.

Pete

 

[Paul]

Loose the finalizer. Implement IDisposable.

Personally I don't see why you need to open the filestream/streamwriter
until you call WriteToStream() in any case as you are buffering in a string
builder. In which case just suround firestream/streamwriter with a using.

I would also create the base of this class abstract and overide the
writetostring functions in a subclass.

Be aware calling the overload of WriteToString could result in a
nullreference exception on stringOut.