0

I'm looking for a good way to dispose of a series of objects, so the problem is in GetMessage if I use a using statement for the underlying stream to create the attachment... in method 1 when I go to send the mail message the stream for the attachment is disposed of and will receive an error. If I take off the using statement it works find but I haven't disposed of DataStream Dest properly. My initial though was adding a parameter List to method one that I can add Dest to each time GetMessage is called and in method 1 at the end of the method loop through the list to dispose of the streams. SendMessage can be called in a loop from a batch process so many streams could potentially be created and I want to leave it to the caller (SendMessage) to dispose of them rather than in GetMessage since SendMessage is where the send takes place and I need to make sure the message has sent before disposal. Any suggestions???

                void sendMessage(MessageType type){
                   MailMessage m = Method2();
                   switch(type)
                   {
                      case type1:
                        m = Object1().GetMessage(param1)
                      case type2:
                        m = Object2().GetMessage(param1, param2)
                      case type3:
                        m = Object3().GetMessage(param1)
                      case type4:
                        m = Object4().GetMessage(param1, param2, param3)
                      case NewType5:
                        m = NewType5().GetMessage(param1, param2)
                   }
                   // this method does the send cause it has the mailmessage
                   m.Send();
                }

                class NewType5()
                {
                MailMessage GetMessage(){

                    // used to be 
                    // using (var Dest = new DataStream(true)){ .... }
                    DataStream Dest = new DataStream(true);

                    // code that reads info into the stream

                    // Go back to the start of the stream
                    Dest.Position = 0;

                    // Create attachment from the stream
                    Attachment mailattachement = new Attachment(Dest, contentType);

                    //Create our return value
                    var message = new MailMessage();
                    message.To.Add(UserInfo.UserDetail.Email);
                    message.Subject = "P&L Data View - " + DateTime.Now.ToString();
                    message.Attachments.Add(mailattachement);
                    return message;
               }
             }

To move send into GetMessage I would have to change all the other object implementations on all the other objects. I would rather let send message take care of the disposal for newtype5 because from a batch process it could be called 500 times. Does this make the example a little more clear. Send Message was already written an implemented against is several other objects with a GetMessage implementation. Mine just happens to use a stream and I can't dispose of the stream before the send takes place.

2
  • There are only 2 ways to do this: either dispose in Method2 or create/dispose the stream in Method1 and inject it as a parameter in Method2From your comments to the answers currently given I can only conclude that (1) you did not provide all information in your question and (2) your design is flawed.
    – jeroenh
    Commented Aug 9, 2012 at 14:25
  • I'm simply adding a new type, much of the code was already written by someone else. But my new type requires attachments acquired through a stream... so I need to alter the caller SendMessage to handle the disposal of the stream from GetMessage on the new Type but only after I know the send takes place. I can't rewrite other implementations of GetMessage on all the other object types. I can't use the stream as a param in GetMessage because again the dispose would get put after the get message call and the send is done outside of the switch statement.
    – DRobertE
    Commented Aug 9, 2012 at 14:55

2 Answers 2

1

Why not just move it to be a param of meth2: ?

            void method1(){
               using (var Dest = new DataStream(true))
               {
                   MailMessage m = Method2(Dest);
                   .....
                   m.Send();
               }
            }

            MailMessage Method2(Stream Dest){

                // code that reads info into the stream

                // Go back to the start of the stream
                Dest.Position = 0;

                // Create attachment from the stream
                Attachment mailattachement = new Attachment(Dest, contentType);

                //Create our return value
                var message = new MailMessage();
                message.To.Add(UserInfo.UserDetail.Email);
                message.Subject = "P&L Data View - " + DateTime.Now.ToString();
                message.Attachments.Add(mailattachement);
                return message;
           }

is your Message.Send() method asynchronous? do you want to parallelize the sending?

9
  • The Method2 is an interface method called from multiple types of objects, other aspects of the mail message are modified in method 1 based on what type of object method 2 is called from. The send has to occur in method 1, and while that would be the obvious choice to have the method creating the attachment send the message, the implementation of method 2 changes with each object type, the caller or method 2 just always knows a mailmessage object is returned and it knows to do the send.
    – DRobertE
    Commented Aug 9, 2012 at 14:13
  • What do you mean by "it knows to do the send" ? How your code that calls the Method2 knows whether the object WAS or WAS NOT sent, if the Method2() takes no control parameters and returns only the message object? Answer to that question dictates why/when/how to dispose the stream. Commented Aug 9, 2012 at 14:18
  • Ok, thanks. Another question: what can you change? Can you make the Send() or GetMessage() methods take a parameter? Or maybe you can edit the MailMessage class or is it that one from System.Net.Mail namespace? Commented Aug 9, 2012 at 18:58
  • It's the Sys.net.mail message class built into .net. I actually can alter the GetMessage to take a List of IDisposeable types and add the streams to it, that way the caller can loop through the list to close all the streams that get opened after all the messages get sent.
    – DRobertE
    Commented Aug 9, 2012 at 21:01
  • GetMessage( List<IDisposable> disposalStreams ) caller of GetMessage now does this... if (disposalObjects.Count > 0) disposalObjects.AsParallel().ForAll(t=> t.Dispose()); Not the most elaborate or elegant solution but it should work for most of the circumstances. I was just hoping to find another solution. My hands were slightly tied since the method is used in so many places. anything else would present a breaking change.
    – DRobertE
    Commented Aug 9, 2012 at 21:02
1

I suggest you do the MailMessage.Send in Method2; with the code you have given there does not seem to be any reason for doing it in the calling method.

2
  • 1
    That would allow the DataStream to be disposed properly in Method2 with a using as well. Commented Aug 9, 2012 at 14:02
  • The Method2 is an interface method called from multiple types of objects, other aspects of the mail message are modified in method 1 based on what type of object method 2 is called from. The send has to occur in method 1, and while that would be the obvious choice to have the method creating the attachment send the message, the implementation of method 2 changes with each object type, the caller or method 2 just always knows a mailmessage object is returned and it knows to do the send.
    – DRobertE
    Commented Aug 9, 2012 at 14:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.