Closed Streamwriter- won't process 2

[DCSage]

I have a small program that allows files to be uploaded, moved, and the evidence written to a text file.

However, I getting the following error:

Error: The files have not been copied or archived. 
System.ObjectDisposedException: Cannot write to a closed TextWriter. at System.IO.__Error.WriterClosed() at System.IO.StreamWriter.Flush(Boolean flushStream, Boolean flushEncoder) at System.IO.StreamWriter.Write(Char[] buffer, Int32 index, Int32 count) at System.IO.TextWriter.WriteLine(String value) at FileCopy.Program.Main(String[] args) in C:\Inetpub\[URL unfurl="true"]wwwroot\BIGFileCopy\FileCopy\FileCopy\Program.cs:line[/URL] 119

I commented out the upload portion because it's not needed for testing, but the move portion should work fine. I have also moved the stw.close around a bit, and I get a few different errors. Where am I going wrong?

static void Main(string[] args)
        {
            //declare variables
            String sourcePath, targetPath, message, Subject;

            string[] emailGroup = new string[3];
            /*places recipient number in emailGroup array*/

            string cis = "MY EMAIL HERE";

            //Directory is the original directory
            DirectoryInfo dir = new DirectoryInfo(@"\\SERVER1\Dealwe\BIG_Logs\");

            //Define Directory Paths
            sourcePath = @"\\SERVER1\Dealwe\BIG_Logs";
            targetPath = @"\\SERVER1\Dealwe\BIG_Logs\Sent to RET";
            FileInfo mi = new FileInfo(sourcePath);

            DateTime currentDate = new DateTime();
            currentDate = DateTime.Now;

            string dateTime = Convert.ToString(mi.CreationTime.ToShortTimeString());
            string fullTime = currentDate.ToString("Mdyy_hmm");

            sendEmail confirmMail = new sendEmail();


            //Write Log files
            StreamWriter stw = new StreamWriter(@"\\SERVER1\Dealwe\TEST_BIG_TG\Sent to HGA\BIG_Logs_" + fullTime + ".txt");
            string somefile = @"\\SERVER1\Dealwe\TEST_BIG_TG\Sent to HGA\BIG_Logs_" + fullTime + ".txt";

            //Prepare Email Headers

            Subject = "New File(s) Successfully Uploaded and Archived!";
            message = "<font face='arial' size='2'>The following files were successfully uploaded to the FTP site and archived on the Server.<br><br><strong>Process Completion Time:</strong>  " + currentDate + "</font><br>";


            //Loop through files moved and write results to text file

            try
            {
                //Find files by extension
                String[] files = Directory.GetFiles(sourcePath, "*.edi");

                for (int i = 0; i < cis.Length; i++)
                {
                    if (files.Length > 0)
                    {
                        //Write to Stream
                        stw.WriteLine("BIG Files successfully uploaded to FTP site and archived on the Server.  Post Time: " + currentDate);
                        stw.WriteLine("");
                        stw.WriteLine("");

                        foreach (String file in files)
                        {
                            //Check if target folder exists.   If it doesn't, create a new target folder. 

                            if (!System.IO.Directory.Exists(targetPath))
                            {
                                System.IO.Directory.CreateDirectory(targetPath);
                            }
                            else
                            {
                                //Grab Basic file information
                                String filename = Path.GetFileName(file);
                                FileInfo fu = new FileInfo(filename);
                                string afile = fu.Name;
                                String dest = Path.Combine(targetPath, filename);


                                                                /*PRODUCTION FTP Info here*/

                                
                                /*comment out for testing*/
                                /*uploadFile myfi = new uploadFile();
                                myfi.Upload(ftpaddress, un, pw, file);
                                 */


                                //Move the file on the same server to the Destination Folder
                                System.IO.File.Move(file, dest);

                                //Write Information to Text File
                                stw.WriteLine("*" + filename);
                                stw.WriteLine("");


                                //Write the same information to email
                                message += "<font face='arial' size='2'><ul><li>" + filename + "</li></ul></font><br>";
                                stw.Close();

                            }
                        }

                        confirmMail.send(message + "<br><br>", cis, Subject, somefile);


                    }
                    else
                    {
                        //Send email if there are no files on the server
                        confirmMail.send("<font face='arial' size='2'>There were no files on the Dealwe server to send.  Process will repeat in an hour.</font>" + "<br><br>", cis, "No Files to Process", "0");
                    }
                    //stw.Close();  
                }
            }
            catch (Exception err)
            {
                confirmMail.send("<font face='arial' size='2'>Error:  The files have not been copied or archived. <br></font>" + err, cis, "Error Processing Files", "0");
            }
        }
    }

thanks in advance.

 

[jmeckley]

I would start with 3 changes.
1. you are not managing the streams properly. they are never disposed of and could potentially never even get disposed because there is no exception management to handle proper disposal. look into the using keyword and how to use the try/finally block to handle proper disposal.
2. too much is happening here. you are trying to get files, write files and send an email. you may have an easier time building, reading, maintaining and debugging the code if you break these into separate members/objects.
3. you should be logging the error. there is no guarantee that send mail will work. you should be logging the error to at least 2, if not 3 locations. a logging library like log4net makes this easy.

the error

Jason Meckley
Programmer
Specialty Bakers, Inc.

faq855-7190
faq732-7259

 

[PGO01]

It is because you create the StreamWriter instance before your loop begins, and then use it and close it IN YOUR LOOP. So on the next iteration, an attempt to use the closed StreamWriter causes an exception to be thrown.

Just some other personal preference bits, I would put the StreamWriter instantiation in a 'using' statement, and make sure you're not doing anything in your catch block that could also throw an error - as this could mask another error.

Also, are you sure this bit of code is correct:

if (!System.IO.Directory.Exists(targetPath))
{
    System.IO.Directory.CreateDirectory(targetPath);
}
else
{
    //Grab Basic file information
    String filename = Path.GetFileName(file);
    ...
...

Or should it be this:

if (!System.IO.Directory.Exists(targetPath))
    System.IO.Directory.CreateDirectory(targetPath);

//Grab Basic file information
String filename = Path.GetFileName(file);
...
...

 

[DCSage]

Thanks for your suggestions on correcting my error.

I already created 2 classes - sendEmail, and uploadFile. I guess that since the app is inhouse, the sendEmail class works fine as well as the uploadFile. From your recommendations, since I currently have the functions to copy and move the file in the main Program class, are you stating that I should break this down into separate classes to copy, upload, and log?
for the following:

if (!System.IO.Directory.Exists(targetPath))
{
    System.IO.Directory.CreateDirectory(targetPath);
}
else
{
    //Grab Basic file information
    String filename = Path.GetFileName(file);

I was tring to write that if the directory doesn't exist, create it. If it does, go ahead amd grab the file information. Please tell me if I am still in the wrong ballpark.

Thank you.

 

[jmeckley]

I guess that since the app is inhouse, the sendEmail class works fine as well as the uploadFile.

why?

since I currently have the functions to copy and move the file in the main Program class, are you stating that I should break this down into separate classes to copy, upload, and log?

I would do something like this

var logger = LogManager.GetLogger(GetType());
try
{
   var source = new Source(...);
   var destination = new Destination(...);
   var notification = EmailNotification(...);

   var result = source.MoveTo(destination);
   notification.Notify(result);
}
catch(Exception e)
{
   logger.Error(e.Message, e);
}

where source will get the files. destination will save the files to a new location and email notification will send the email. the result of source.MoveTo will return an object that EmailNotification will use to construct the email.

The ellipsis represent any ctor args that may be required for the object to work. for example the source root path, destination root path, email server, email port, etc.

LogManager is part of log4net. log4net can then be configured to log messages anywhere you need.

Jason Meckley
Programmer
Specialty Bakers, Inc.

faq855-7190
faq732-7259

 

[PGO01]

I was tryng to write that if the directory doesn't exist, create it. If it does, go ahead and grab the file information. Please tell me if I am still in the wrong ballpark.

Using your code, if the directory does exist, the code in the 'else' block will not get processed. I think you want the code in the 'else' block processed either way. If this is right, keep the 'if' part that creates the directory if it does not exist, but remove the 'else' and relating braces so that that part of the code gets processed all the time.

 

[DCSage]

thank you all for your help.

My app works now. I created/revised the following classes as well as my try/catch statements:

1) sendEmail.cs
2) uploadFile.cs
3) moveFile.cs
4) Program.cs


I am included the 'USING' keyword, which really seemed to make a difference and placed my code in separate members/objects too.

Thank You so much for your pointers!!!!!!!!!!!!!!!!!!!!!!!!!!