none
implement IDisposable issue

    Question

  • Hello everyone,

    I have a static class which wraps a file stream object. And I want to implement the Dispose pattern. I define an uninitialize method in the static class which call Dispose explicitly with parameter value true. Here is my code,

    My questions are,

    1. Normally there should be a destructor in a class which calls Dispose with parameter value false, but for a static class, there is no concept like destructor, how to implement calling Dispose with false parameter?

    2. When the Dispose method without parameter will be called?

    3. Is it correct to call GC.SuppressFinalize(this)? Since for a static class, there is no "this" object?

    4. Correct to define disposed as static field?

    5. Are there anything wrong with my code?

    [Code]
            static private StreamWriter currentLogStream;

            private static bool disposed = false;

            public void Dispose()
            {
                Dispose(true);
                GC.SuppressFinalize(this);
            }

            private static void Dispose(bool disposing)
            {
                // Check to see if Dispose has already been called.
                if (false == disposed)
                {
                    // If disposing equals true, dispose all managed
                    // and unmanaged resources.
                    if (disposing)
                    {
                        // Dispose managed resources.
                        currentLogStream.Dispose();
                    }
                }
                disposed = true;
            }

            public static void Uninitialize()
            {
                Dispose(true);
            }
    [/Code]

    thanks in advance,
    George

    Wednesday, November 26, 2008 12:58 PM

Answers

  • http://msdn.microsoft.com/en-us/library/system.idisposable.dispose.aspx

    Do you know how to test it out yourself?  I don't.  The GC does its' thing when it gets ready to do it.  This interface is most commonly implemented on generic classes. Unless this object is unusally large, what would be the point of doing it on a singleton?  A class named Logger sounds like it could be rather large, but the name sounds more like something that "does" soemthing more so than something that "is" something.

    Rudedog   =|^D
    Mark the best replies as answers. "Fooling computers since 1971."
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:37 AM
    Wednesday, November 26, 2008 2:45 PM
  • In short, the answer is "Don't".

    To clarify, don't implement the dispose pattern with a static class, because it means nothing in that context.  

    1.  Static classes cannot implement interfaces (including IDisposable) at all.  All interface implementations require an instance class, and the methods defined in the interfaces are always instance methods.  There is no such thing as a static destructor either.  The whole idea of Dispose for a static class makes no sense at all.  You have no instance to dispose of. 

    2. It won't be called.

    3. No.  There is no "this" to pass into Dispose().

    4. You have no choice but to define dispose as a static field, because you're in a static class, nevertheless, the whole implementation makes no sense, so the static field means nothing.

    5. Implementing IDisposable only works with an instance, non-static class.  The Dispose() method must be implemented as a non-static method.

    If I try this:

    public static class MyStaticDisposable : IDisposable  
    {  

    I will get a compile error saying "static classes cannot implement interfaces". 

    Don't create static streams or static streamwriter's unless you have a very good reason to do so.  Currently, I can't think of any.  If you're going to use a static class, instantiate a brand new streamwriter in the methods you intend to use it in, and dispose of it before the end of the method.  Use a using statement, like this:

    public static void DoSomething()  
    {  
        using (StreamWriter writer = new StreamWriter(File.Open(@"C:\myfile.txt", FileMode.OpenOrCreate)))  
        {  
            writer.Write("Something");  
        }  

    Why would you want to implement an IDisposable pattern with a static class anyways?  If you use the class, you'll only be able to use it once.  Once you set disposed to true, you can't "unset" it, and you can't instantiate a new version, so you'll be stuck with a useless piece of code for the lifetime of your application. 

    In short, ditch the whole idea, and save yourself lots of time.  
    David Morton - http://blog.davemorton.net/
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:37 AM
    Wednesday, November 26, 2008 2:48 PM
  • Just read your other post about the fact that you're implementing a singleton.

    Again, don't do it.  If you dispose of the only accessible instance of the class, there's no way you can get another instance.  That makes no sense whatsoever.  If you want to implement IDisposable, don't make it a singleton and don't make it static.  Make it a standard, plain vanilla class that happens to implement IDisposable. 

    Your specific implementation also won't work, because you've defined Dispose() as a private, static method.  All implementations of IDisposable must create an instance method called Dispose().  Static doesn't count.

    Back to the other issue, however, I wouldn't keep a StreamReader or StreamWriter open indefinately anyways.  Chances are that doing so is grabbing the handle on some file somewhere on your system.  Open the stream only when you need to write or read to it.  The rest of the time, make sure they're all disposed. 
    David Morton - http://blog.davemorton.net/
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Wednesday, November 26, 2008 2:55 PM
  • You're right on a couple of accounts.  I was moving too quickly through your posts.

    1. It's fine to create the logger as a singleton, but I still wouldn't make the StreamWriter static.  I'd instantiate it within your "Write" or "Log" method (or whatever method you call to write to the log), and in that same method, I'd close the Stream.  I still wouldn't leave the stream open for a long time.  If you use the using statement, and don't create a class-level StreamWriter, you won't have to make your class Disposable, and you could still make it singleton, and everyone is happy.  It's the simplest implementation.

    2. If you follow my suggestion in number 1, you won't have any problems with the application failing during initialization.  In other words, your attempt to solve the issue is creating the very issue it's trying to solve.  Don't try to solve the issue with IDisposable, and instead declare each stream as a method level variable, Disposing of it within the method, and you won't have to implement IDisposable to begin with.

    3. You're right.  I didn't see the public Dispose method you had implemented. 

    I think a better (and simpler) implementation would be something like this:

    public static class Logger  
    {  
        public static void Log(string text)  
        {  
            using (StreamWriter sw = new StreamWriter(File.Open("C:\log.txt", FileMode.OpenCreate)))  
            {  
                sw.WriteLine(text);  
            }  
        }  

    Now, all you have to call is:

    Logger.Log("something really bad happened, and I need to panic now.");

    Your stream is cleaned up, everything is hunky-dory, and you have no problems, even if the application fails to initialize properly. 
    David Morton - http://blog.davemorton.net/
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Wednesday, November 26, 2008 3:35 PM
  • George2 said:

    Thanks Rudedog!

    1.

    "but the name sounds more like something that "does" soemthing more so than something that "is" something." -- I am confused about your points. Could you say in some other words please? :-)

    2.

    My purpose is to have only one instance of type Logger in the application, when the application starts, it initialize and class and create one instance, and when the application ends, it uninitialize the instance. The reason why implement Dispose pattern is, I am afraid of the application fails during initialization, and if fails there, if I do not implement the Dispose pattern, I am afraid the Logger instance can not be freed cleanly (one file handle wrapped in the file stream will leak??). Any comments?

    regards,
    George



    1.  Like David, I was questioning as to why the need to implement the interface at all.  There were no details about your Logger class posted when I raised that point.  The class name "Logger" suggests a class that provides methods for use by consumers.  The "Logger" does something, it provides methods that you described.

     The name "Logger" suggests that it works with a class named "Log", which I would guess to be the files that read and write to disk.  The "Log" is a thing, or a class that describes an object.  "Logger" is not a thing, the name souinds like a tool to use on Logs.

    My point was why whould you need or want to dispose of a tool.  I can understand creating only one instance of the tool during the lifetime of your application.  As David noted, so why dispose of it at all.  You might not get another one back again.

    A class of static methods would serve your purpose very nicely.  As for your description of the heavy file I/O, that is what database files are really good at doing.

    Rudedog   =|^D
    Mark the best replies as answers. "Fooling computers since 1971."
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Wednesday, November 26, 2008 4:38 PM
  •  The point of a singleton class is to not have static members.
    It's the instance of this class that should be static.
    Puzzles, brain teases, riddles, enigmas: http://www.toysforthebrain.com
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Wednesday, November 26, 2008 5:43 PM
  • ThE_lOtUs said:

     The point of a singleton class is to not have static members.
    It's the instance of this class that should be static.


    Puzzles, brain teases, riddles, enigmas: http://www.toysforthebrain.com



    I agree.  My point was similar to David's  do away with IDisposable if the class is simply a collection of methods, which it sounds like it.  Just mark the methods, or even the class, as static and go from there. 

    The OP probably needs to worry more about the disposing of the objects that Logger creates and uses than the class itself.

    Rudedog   =|^D
    Mark the best replies as answers. "Fooling computers since 1971."
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Wednesday, November 26, 2008 5:53 PM
  •  If you have a static class St and a method A inside you do this:

    St.A ( ) again and again. You just reuse the same MIL code.

    You should reread the entire thread. Static cannot be disposed of. That's what they have been telling you all along. You should rewrite your code.
    AlexB
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Thursday, November 27, 2008 2:43 PM
  • 1.  If it is similar to the System.Diagnostics.Trace class, I still wonder why you want to implement IDisposable.  The Trace class is sealed and contains a number of static mthods and properties.  It is a tool, which performs functions of disposable objects.

    2. It sounds like you need to separate your classes from one another.  The wrapper and the tool should be separate classes.

    3.  Is the Log class separate from Logger?  If so, good.  Again a class of static methods would not consume additional memory every time you create a new Log object.  Singleton objects behave a little differently from static objects.  Which one to use can vary.

    4.  I have not tested it, but at first glance it appears to follow the sample at the link I provided.  Are you getting any errors?  But, there is no need or reason to dispose of static objects.  Static objects are created when they are first called in code.  The same object gets re-used until the application closes.  It is your actual Log objects that sound like they need disposing.

    What initialization would static methods need?  If you need to initialilze static properties, then something may be flawed in your design.  Those properties should reflect the results or state of the static methods.  Your Log object would need to be initialized in some way no doubt.  A static builder method in your Logger class could do that for you.

    Rudedog   =|^D
    Mark the best replies as answers. "Fooling computers since 1971."
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Thursday, November 27, 2008 2:46 PM
  •  

    If you are using one instance of this object, you don't need a dispose function (unless you are using a win32 object).

    Here are 3 way of doing it.
    1- Singleton
    2- Static
    3- Singleton with one instance of Stream (I do not like this).

    If you really want a Dispose, then use method 1 and don't put it static.

    (This was converted from vb)

    using System.IO;   
    using System.Threading;   
     
    public class Logger1   
    {   
          
        private static Logger1 _instance;   
        private static Mutex _mutexLogger = new Mutex();   
        private static Mutex _mutexWrite = new Mutex();   
          
        public static Logger1 GetInstance()   
        {   
              
            _mutexLogger.WaitOne();   
              
            try {   
                if (_instance == null) {   
                    _instance = new Logger1();   
                }   
            }   
            finally {   
                _mutexLogger.ReleaseMutex();   
            }   
              
            return _instance;   
        }   
          
        public void WriteLog(string line)   
        {   
              
            _mutexWrite.WaitOne();   
              
            try {   
                using (StreamWriter logStream = new StreamWriter("c:\\temp\\log1.txt")) {   
                    logStream.Write(line);   
                }   
            }   
            finally {   
                _mutexWrite.ReleaseMutex();   
            }   
              
        }   
    }   
     
     


     

    using System.IO;   
    using System.Threading;   
     
    public class Logger2   
    {   
          
        private static Mutex _mutexWrite = new Mutex();   
          
        public static void WriteLog(string line)   
        {   
              
            _mutexWrite.WaitOne();   
              
            try {   
                using (StreamWriter logStream = new StreamWriter("c:\\temp\\log2.txt")) {   
                    logStream.Write(line);   
                }   
            }   
            finally {   
                _mutexWrite.ReleaseMutex();   
            }   
              
        }   
    }   
     
     



     

    using System.IO;   
    using System.Threading;   
     
    public class Logger3   
    {   
          
        private static Logger3 _instance;   
        private static Mutex _mutexLogger = new Mutex();   
        private static Mutex _mutexWrite = new Mutex();   
          
        private StreamWriter _logStream;   
          
        public static Logger3 GetInstance()   
        {   
              
            _mutexLogger.WaitOne();   
              
            try {   
                if (_instance == null) {   
                    _instance = new Logger3();   
                }   
            }   
            finally {   
                _mutexLogger.ReleaseMutex();   
            }   
              
            return _instance;   
        }   
          
        public Logger3()   
        {   
              
            //' Disposed on logStream will be called when this class get destroyed   
            _logStream = new StreamWriter("c:\\temp\\log3.txt");   
              
        }   
          
        public void WriteLog(string line)   
        {   
              
            _logStream.Write(line);   
              
        }   
    }   
     
     


     

        Logger1.GetInstance().WriteLog("Test 1");   
        Logger2.WriteLog("Test 2");   
        Logger3.GetInstance().WriteLog("Test 3");   
     

    Puzzles, brain teases, riddles, enigmas: http://www.toysforthebrain.com
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Thursday, November 27, 2008 3:59 PM
  • IOtUs,
    I think the OP might be trying to do too much with his Logger class, which is why he wants to Dispose() of it.
    class Log : IDisposable  
    {  
    }  
     
    class Logger  
    {  
        public Log CreateLogInstance(LogSpec logSpec);  
        public void WriteLog(Log log);  

    So he could use it like this.

    Log newLog = Logger.CreateLogInstance(logSpec);  
    Logger.WriteLog(newLog);  
    //  newLog.Dispose(); 

    Rudedog   =|^D
    Mark the best replies as answers. "Fooling computers since 1971."
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Thursday, November 27, 2008 4:57 PM

All replies

  •  Static is static. I don't think you can get rid of it once you defined it and the app has been instantiated.
    AlexB
    Wednesday, November 26, 2008 2:29 PM
  • Sorry AlexB, I am wrong. The class is not static, but singleton (only one instance of the class will be created). I posted all of my code below, could you help to review whether my code is correctly implementing the Dispose pattern please? :-)

    class Logger : IDisposable
    {
            private static bool disposed = false;

            public void Dispose()
            {
                Dispose(true);
                GC.SuppressFinalize(this);
            }

            ~Logger()
            {
                Dispose(false);
            }

            private static void Dispose(bool disposing)
            {
                // Check to see if Dispose has already been called.
                if (false == disposed)
                {
                    // If disposing equals true, dispose all managed
                    // and unmanaged resources.
                    if (disposing)
                    {
                        // Dispose managed resources.
                        currentLogStream.Dispose();
                    }
                }
                disposed = true;
            }

            public static void Uninitialize()
            {
                Dispose(true);
            }
    }


    regards,
    George

    Wednesday, November 26, 2008 2:33 PM
  • http://msdn.microsoft.com/en-us/library/system.idisposable.dispose.aspx

    Do you know how to test it out yourself?  I don't.  The GC does its' thing when it gets ready to do it.  This interface is most commonly implemented on generic classes. Unless this object is unusally large, what would be the point of doing it on a singleton?  A class named Logger sounds like it could be rather large, but the name sounds more like something that "does" soemthing more so than something that "is" something.

    Rudedog   =|^D
    Mark the best replies as answers. "Fooling computers since 1971."
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:37 AM
    Wednesday, November 26, 2008 2:45 PM
  • In short, the answer is "Don't".

    To clarify, don't implement the dispose pattern with a static class, because it means nothing in that context.  

    1.  Static classes cannot implement interfaces (including IDisposable) at all.  All interface implementations require an instance class, and the methods defined in the interfaces are always instance methods.  There is no such thing as a static destructor either.  The whole idea of Dispose for a static class makes no sense at all.  You have no instance to dispose of. 

    2. It won't be called.

    3. No.  There is no "this" to pass into Dispose().

    4. You have no choice but to define dispose as a static field, because you're in a static class, nevertheless, the whole implementation makes no sense, so the static field means nothing.

    5. Implementing IDisposable only works with an instance, non-static class.  The Dispose() method must be implemented as a non-static method.

    If I try this:

    public static class MyStaticDisposable : IDisposable  
    {  

    I will get a compile error saying "static classes cannot implement interfaces". 

    Don't create static streams or static streamwriter's unless you have a very good reason to do so.  Currently, I can't think of any.  If you're going to use a static class, instantiate a brand new streamwriter in the methods you intend to use it in, and dispose of it before the end of the method.  Use a using statement, like this:

    public static void DoSomething()  
    {  
        using (StreamWriter writer = new StreamWriter(File.Open(@"C:\myfile.txt", FileMode.OpenOrCreate)))  
        {  
            writer.Write("Something");  
        }  

    Why would you want to implement an IDisposable pattern with a static class anyways?  If you use the class, you'll only be able to use it once.  Once you set disposed to true, you can't "unset" it, and you can't instantiate a new version, so you'll be stuck with a useless piece of code for the lifetime of your application. 

    In short, ditch the whole idea, and save yourself lots of time.  
    David Morton - http://blog.davemorton.net/
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:37 AM
    Wednesday, November 26, 2008 2:48 PM
  • Just read your other post about the fact that you're implementing a singleton.

    Again, don't do it.  If you dispose of the only accessible instance of the class, there's no way you can get another instance.  That makes no sense whatsoever.  If you want to implement IDisposable, don't make it a singleton and don't make it static.  Make it a standard, plain vanilla class that happens to implement IDisposable. 

    Your specific implementation also won't work, because you've defined Dispose() as a private, static method.  All implementations of IDisposable must create an instance method called Dispose().  Static doesn't count.

    Back to the other issue, however, I wouldn't keep a StreamReader or StreamWriter open indefinately anyways.  Chances are that doing so is grabbing the handle on some file somewhere on your system.  Open the stream only when you need to write or read to it.  The rest of the time, make sure they're all disposed. 
    David Morton - http://blog.davemorton.net/
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Wednesday, November 26, 2008 2:55 PM
  • Hi David,


    Two more comments, not agree all of your words. :-)

    1.

    "If you dispose of the only accessible instance of the class, there's no way you can get another instance.  That makes no sense whatsoever." -- my purpose is to have only one instance of type Logger in the application, when the application starts, it initialize and class and create one instance, and when the application ends, it uninitialize the instance.

    What do you mean "no way you can get another instance"? Actually I do not need another instance for the same class in an application when the application starts.

    2.

    My purpose to implement Dispose pattern is, I am afraid of the application fails during initialization, and if fails there, if I do not implement the Dispose pattern, I am afraid the Logger instance can not be freed cleanly. Any comments?

    3.

    "Your specific implementation also won't work, because you've defined Dispose() as a private, static method.  All implementations of IDisposable must create an instance method called Dispose().  Static doesn't count." -- No. Disagree here. I have implemented Dispose which is public (the one without parameter), which is defined in IDisposable interface. The one I marked with private is with bool parameter, which is not defined in IDisposable interface. Any comments?

    regards,
    George
    Wednesday, November 26, 2008 3:18 PM
  • Thanks Rudedog!

    1.

    "but the name sounds more like something that "does" soemthing more so than something that "is" something." -- I am confused about your points. Could you say in some other words please? :-)

    2.

    My purpose is to have only one instance of type Logger in the application, when the application starts, it initialize and class and create one instance, and when the application ends, it uninitialize the instance. The reason why implement Dispose pattern is, I am afraid of the application fails during initialization, and if fails there, if I do not implement the Dispose pattern, I am afraid the Logger instance can not be freed cleanly (one file handle wrapped in the file stream will leak??). Any comments?

    regards,
    George
    Wednesday, November 26, 2008 3:21 PM
  • You're right on a couple of accounts.  I was moving too quickly through your posts.

    1. It's fine to create the logger as a singleton, but I still wouldn't make the StreamWriter static.  I'd instantiate it within your "Write" or "Log" method (or whatever method you call to write to the log), and in that same method, I'd close the Stream.  I still wouldn't leave the stream open for a long time.  If you use the using statement, and don't create a class-level StreamWriter, you won't have to make your class Disposable, and you could still make it singleton, and everyone is happy.  It's the simplest implementation.

    2. If you follow my suggestion in number 1, you won't have any problems with the application failing during initialization.  In other words, your attempt to solve the issue is creating the very issue it's trying to solve.  Don't try to solve the issue with IDisposable, and instead declare each stream as a method level variable, Disposing of it within the method, and you won't have to implement IDisposable to begin with.

    3. You're right.  I didn't see the public Dispose method you had implemented. 

    I think a better (and simpler) implementation would be something like this:

    public static class Logger  
    {  
        public static void Log(string text)  
        {  
            using (StreamWriter sw = new StreamWriter(File.Open("C:\log.txt", FileMode.OpenCreate)))  
            {  
                sw.WriteLine(text);  
            }  
        }  

    Now, all you have to call is:

    Logger.Log("something really bad happened, and I need to panic now.");

    Your stream is cleaned up, everything is hunky-dory, and you have no problems, even if the application fails to initialize properly. 
    David Morton - http://blog.davemorton.net/
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Wednesday, November 26, 2008 3:35 PM
  •  

    I taught disposed was used when you wanted to clear memory right away that was not in the GC (like a win32 dll).

     

    Ex: If you create and throw away a lot of Logger objects then you might want to clear that memory (that is not in the GC, GC memory is cleared by itself).

     

    In your case, with a singleton, I’m pretty sure that putting your memory clear logic in the deconstructor is good enough.

     

    Now, if your class is not static then question 1, 3 is irrelevant.

     

    2: You have to call it yourself, that's the point of Dispose

    4: You'll have to convert the static class to a singleton

    5: Look up Rudedog2 link

     

    Since logger is a singleton. You should remove everything that is static in your class and take a look at: http://msdn.microsoft.com/en-us/library/ms998558.aspx (as you see, it’s the logger object that should be static, not the class).

     

    Hope this help!

     

    (The post right before me has a good solution).


    Puzzles, brain teases, riddles, enigmas: http://www.toysforthebrain.com
    Wednesday, November 26, 2008 3:38 PM
  • Hi David,

    I think different design fit into different scenarios. The reason why I want to keep the file open is, the write operation of the Logger class is very heavy, i.e. write operation happens very frequently. I want to save open and close stream operations.

    My question is in my design (Singleton class with static member to store stream object instance), whether I am correctly implemented the Dispose pattern?

    regards,
    George
    Wednesday, November 26, 2008 3:57 PM
  • George2 said:

    Thanks Rudedog!

    1.

    "but the name sounds more like something that "does" soemthing more so than something that "is" something." -- I am confused about your points. Could you say in some other words please? :-)

    2.

    My purpose is to have only one instance of type Logger in the application, when the application starts, it initialize and class and create one instance, and when the application ends, it uninitialize the instance. The reason why implement Dispose pattern is, I am afraid of the application fails during initialization, and if fails there, if I do not implement the Dispose pattern, I am afraid the Logger instance can not be freed cleanly (one file handle wrapped in the file stream will leak??). Any comments?

    regards,
    George



    1.  Like David, I was questioning as to why the need to implement the interface at all.  There were no details about your Logger class posted when I raised that point.  The class name "Logger" suggests a class that provides methods for use by consumers.  The "Logger" does something, it provides methods that you described.

     The name "Logger" suggests that it works with a class named "Log", which I would guess to be the files that read and write to disk.  The "Log" is a thing, or a class that describes an object.  "Logger" is not a thing, the name souinds like a tool to use on Logs.

    My point was why whould you need or want to dispose of a tool.  I can understand creating only one instance of the tool during the lifetime of your application.  As David noted, so why dispose of it at all.  You might not get another one back again.

    A class of static methods would serve your purpose very nicely.  As for your description of the heavy file I/O, that is what database files are really good at doing.

    Rudedog   =|^D
    Mark the best replies as answers. "Fooling computers since 1971."
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Wednesday, November 26, 2008 4:38 PM
  •  The point of a singleton class is to not have static members.
    It's the instance of this class that should be static.
    Puzzles, brain teases, riddles, enigmas: http://www.toysforthebrain.com
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Wednesday, November 26, 2008 5:43 PM
  • ThE_lOtUs said:

     The point of a singleton class is to not have static members.
    It's the instance of this class that should be static.


    Puzzles, brain teases, riddles, enigmas: http://www.toysforthebrain.com



    I agree.  My point was similar to David's  do away with IDisposable if the class is simply a collection of methods, which it sounds like it.  Just mark the methods, or even the class, as static and go from there. 

    The OP probably needs to worry more about the disposing of the objects that Logger creates and uses than the class itself.

    Rudedog   =|^D
    Mark the best replies as answers. "Fooling computers since 1971."
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Wednesday, November 26, 2008 5:53 PM
  • Thanks Rudedog!

    1.

    The logger class works similarly as .Net Trace class, whcih write log into disk for an application. The log is provided in the form of a DLL. The write operation is heavy;

    2.

    "My point was why whould you need or want to dispose of a tool." -- because it wraps Disposable file stream member;

    3.

    "A class of static methods would serve your purpose very nicely" -- Dispose method from IDisposable interface is not static, so there must be an instance of Log class.

    4.

    Here is my code, could you help to review whether it is correct or any potential issues?

    class Logger : IDisposable
    {
            private static bool disposed = false;
            private static StreamWriter  logStream;

            public void Dispose()
            {
                Dispose(true);
                GC.SuppressFinalize(this);
            }

            ~Logger()
            {
                Dispose(false);
            }

            private static void Dispose(bool disposing)
            {
                // Check to see if Dispose has already been called.
                if (false == disposed)
                {
                    // If disposing equals true, dispose all managed
                    // and unmanaged resources.
                    if (disposing)
                    {
                        // Dispose managed resources.
                        LogStream.Dispose();
                        LogStream = null;
                    }
                }
                disposed = true;
            }

            public static void Uninitialize()
            {
                Dispose(true);
                GC.SuppressFinalize(l ogStream);
            }
    }


    regards,
    George

    Thursday, November 27, 2008 8:46 AM
  • Why "The point of a singleton class is to not have static members."? Could you show me the reason and benefits?

    regards,
    George
    Thursday, November 27, 2008 8:47 AM
  • Thanks Rudedog,

    If I made the class static, then I can not control when it is initialized. I want to control the initialize order and step. For example, I want to read log level from app.config, then create a new instance of the Log object and set proper level -- you can see here is sequence and dependencies.

    If the class is static, how could I ensure the sequence?

    regards,
    George
    Thursday, November 27, 2008 8:50 AM
  •  If you have a static class St and a method A inside you do this:

    St.A ( ) again and again. You just reuse the same MIL code.

    You should reread the entire thread. Static cannot be disposed of. That's what they have been telling you all along. You should rewrite your code.
    AlexB
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Thursday, November 27, 2008 2:43 PM
  • 1.  If it is similar to the System.Diagnostics.Trace class, I still wonder why you want to implement IDisposable.  The Trace class is sealed and contains a number of static mthods and properties.  It is a tool, which performs functions of disposable objects.

    2. It sounds like you need to separate your classes from one another.  The wrapper and the tool should be separate classes.

    3.  Is the Log class separate from Logger?  If so, good.  Again a class of static methods would not consume additional memory every time you create a new Log object.  Singleton objects behave a little differently from static objects.  Which one to use can vary.

    4.  I have not tested it, but at first glance it appears to follow the sample at the link I provided.  Are you getting any errors?  But, there is no need or reason to dispose of static objects.  Static objects are created when they are first called in code.  The same object gets re-used until the application closes.  It is your actual Log objects that sound like they need disposing.

    What initialization would static methods need?  If you need to initialilze static properties, then something may be flawed in your design.  Those properties should reflect the results or state of the static methods.  Your Log object would need to be initialized in some way no doubt.  A static builder method in your Logger class could do that for you.

    Rudedog   =|^D
    Mark the best replies as answers. "Fooling computers since 1971."
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Thursday, November 27, 2008 2:46 PM
  •  

    If you are using one instance of this object, you don't need a dispose function (unless you are using a win32 object).

    Here are 3 way of doing it.
    1- Singleton
    2- Static
    3- Singleton with one instance of Stream (I do not like this).

    If you really want a Dispose, then use method 1 and don't put it static.

    (This was converted from vb)

    using System.IO;   
    using System.Threading;   
     
    public class Logger1   
    {   
          
        private static Logger1 _instance;   
        private static Mutex _mutexLogger = new Mutex();   
        private static Mutex _mutexWrite = new Mutex();   
          
        public static Logger1 GetInstance()   
        {   
              
            _mutexLogger.WaitOne();   
              
            try {   
                if (_instance == null) {   
                    _instance = new Logger1();   
                }   
            }   
            finally {   
                _mutexLogger.ReleaseMutex();   
            }   
              
            return _instance;   
        }   
          
        public void WriteLog(string line)   
        {   
              
            _mutexWrite.WaitOne();   
              
            try {   
                using (StreamWriter logStream = new StreamWriter("c:\\temp\\log1.txt")) {   
                    logStream.Write(line);   
                }   
            }   
            finally {   
                _mutexWrite.ReleaseMutex();   
            }   
              
        }   
    }   
     
     


     

    using System.IO;   
    using System.Threading;   
     
    public class Logger2   
    {   
          
        private static Mutex _mutexWrite = new Mutex();   
          
        public static void WriteLog(string line)   
        {   
              
            _mutexWrite.WaitOne();   
              
            try {   
                using (StreamWriter logStream = new StreamWriter("c:\\temp\\log2.txt")) {   
                    logStream.Write(line);   
                }   
            }   
            finally {   
                _mutexWrite.ReleaseMutex();   
            }   
              
        }   
    }   
     
     



     

    using System.IO;   
    using System.Threading;   
     
    public class Logger3   
    {   
          
        private static Logger3 _instance;   
        private static Mutex _mutexLogger = new Mutex();   
        private static Mutex _mutexWrite = new Mutex();   
          
        private StreamWriter _logStream;   
          
        public static Logger3 GetInstance()   
        {   
              
            _mutexLogger.WaitOne();   
              
            try {   
                if (_instance == null) {   
                    _instance = new Logger3();   
                }   
            }   
            finally {   
                _mutexLogger.ReleaseMutex();   
            }   
              
            return _instance;   
        }   
          
        public Logger3()   
        {   
              
            //' Disposed on logStream will be called when this class get destroyed   
            _logStream = new StreamWriter("c:\\temp\\log3.txt");   
              
        }   
          
        public void WriteLog(string line)   
        {   
              
            _logStream.Write(line);   
              
        }   
    }   
     
     


     

        Logger1.GetInstance().WriteLog("Test 1");   
        Logger2.WriteLog("Test 2");   
        Logger3.GetInstance().WriteLog("Test 3");   
     

    Puzzles, brain teases, riddles, enigmas: http://www.toysforthebrain.com
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Thursday, November 27, 2008 3:59 PM
  • IOtUs,
    I think the OP might be trying to do too much with his Logger class, which is why he wants to Dispose() of it.
    class Log : IDisposable  
    {  
    }  
     
    class Logger  
    {  
        public Log CreateLogInstance(LogSpec logSpec);  
        public void WriteLog(Log log);  

    So he could use it like this.

    Log newLog = Logger.CreateLogInstance(logSpec);  
    Logger.WriteLog(newLog);  
    //  newLog.Dispose(); 

    Rudedog   =|^D
    Mark the best replies as answers. "Fooling computers since 1971."
    • Marked as answer by Harry Zhu Wednesday, December 03, 2008 10:38 AM
    Thursday, November 27, 2008 4:57 PM