0

I currently have a class written in C# which does some simple error logging stuff. It's been used quite extensively throughout my project.

class LogError
{
    //constructor
    public LogError() { }

    //public methods
    public void saveToTextFile() { }

    //other similar methods...
}

But, it doesn't seems a good design of this class, as every time I have to create an instance of LogError class before saveToTextFile() method can be used.

So I am thinking of re-designing this class. Would this be a good case to apply Singleton pattern? What about making it static class instead? Any suggestions? Thanks.

woodykiddy
  • 5,622
  • 13
  • 49
  • 92
  • You can also have static methods in a non-static class. – Tieson T. Mar 01 '13 at 05:34
  • what's you project? Web application or desktop application? – Jobert Enamno Mar 01 '13 at 05:34
  • it's desktop app with c# winform. – woodykiddy Mar 01 '13 at 06:26
  • what makes you think that singleton would be a good pattern? – Default Mar 01 '13 at 07:28
  • the pattern is discussed [here](http://jalf.dk/blog/2010/03/singletons-solving-problems-you-didnt-know-you-never-had-since-1995/) and [here](http://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons) and [here](http://stackoverflow.com/questions/1008019/c-singleton-design-pattern) and [here](http://stackoverflow.com/questions/1020312/are-singletons-really-that-bad) and [here](http://blogs.msdn.com/b/scottdensmore/archive/2004/05/25/140827.aspx) and [here](http://programmers.stackexchange.com/questions/40373/so-singletons-are-bad-then-what) – Default Mar 01 '13 at 07:31
  • I don't know if singleton would be a good pattern or not, which is why I am asking the question here. I just wanted to see some brainstorming going. – woodykiddy Mar 04 '13 at 07:13

6 Answers6

1

The problem with Singleton is that it's hard to use different logging behaviour. Image you want to introduce a "Send an email instead of write to text file" later. It's basically the same if you have

new LogError().DoSomething();

or

LogError.Instance.DoSomething();

except for performance and/or implementation of the LogError class itself.

If you want to add flexibility, you'd better use Dependency Injection (which makes your code clearer than with the Singleton, in addition) or Dependency Lookup (which is somewhere in between).

Philipp
  • 10,577
  • 5
  • 57
  • 111
  • +1 for Depencency Injection. To add on that for anyone who's interested you can inject an interface which later on will be much easier to test (because you can mock it). Or for that matter exchange the whole implementation, as long as the contract of the interface is still there – Default Mar 01 '13 at 08:09
1

I would look at Apache log4net. You don't have to worry about anything. You can configure it to log to multiple targets from your configuration file (or in code). The log message template is fully customizable. You can filter and route different log levels (debug/info/warning/error). It's really not worth reinventing the wheel here.

Steven Padfield
  • 584
  • 1
  • 4
  • 12
  • You'd still have to create the `ILog log ..` in every class. Although, I do agree with you. Let's use well tested and generally used frameworks instead of creating our own. (And on a side note, he would probably have to worry about *something* :)) – Default Mar 01 '13 at 08:12
  • Hey, if a developer never exaggerates, something's wrong... ;) – Steven Padfield Mar 03 '13 at 18:37
0

Yes make it singleton and also thread safe

Pradheep
  • 2,973
  • 21
  • 32
0

If you are using , any container ( Autofac, Unity etc) then you can make use of the container.

Singleton can be broken ( By using Reflection so be informed )

one of the implementation would be ( this would not required explicit locking )

public class  MySingleton
{
    private static readonly MySingleton _singtonInstance =  new MySingleton();
    private MySingleton()
    {

    }

    public static MySingleton SingtonInstance
    {
        get { return _singtonInstance; }
    }
}
TalentTuner
  • 16,603
  • 5
  • 35
  • 59
0

You can use interface as your log system facade, like

interface ILoggerFacade{
    void Error(Exception e);
    void Warning(Exception e);
    ....
}

after that you need to make interface implementation

class SimpleLogger:ILoggerFacade{

    void Error(Exception e){//logging error};
    ...
}

and finnaly you need enter point to your logger. I ussually use static class but singleton is variant also.

static class sample:

 class StaticLogger{

     private ILoggerFacade _logger;
     StaticLogger(){
        //choose ILoggerFacade implementation
        _logger=new SimpleLogger();
     }
     public static ILoggerFacade Logger{
        get{ return _logger;}
     }
 }

If you will use facade interface you can easy change loggers in your project if it will be need.

Frank59
  • 2,783
  • 2
  • 29
  • 44
0

There is a solution where you have a logging method that is being called once there are exceptions happen anywhere in your application. All you need to have is a general or common exception handler. Here's how.

On your Program.cs (inside your Main() method before the Application.Run) add this code.

 Application.ThreadException += CommonExceptionHandler;

Create CommonExceptionHandler event on your Program.cs file let's say next to Main method.

private static void CommonExceptionHandler(object sender, ThreadExceptionEventArgs t)
        {
           LogError(t.Exception);
        }

Create LogError Method on your Program.cs

 public static void LogError(Exception ex)
        {
            var errMsg = ex.Message;
            errMsg += ex.InnerException != null ? ex.InnerException.Message : string.Empty;
            //TODO: Do what you want if an error occurs
        }

This will catch all exceptions occur in your application. You don't need to worry anymore whether you would call your error log class for every catch block in all of your methods

Jobert Enamno
  • 4,203
  • 7
  • 36
  • 60