Function? Class?

Question

I'm writing an app (C#) and at times I will need to log to the Windows event log. So the first thing that comes to mind is to write a function in my one and only class and call it when I need it. Something like this:

private void Write_Event_Log(string log, string source, string message, EventLogEntryType type, int eventid)
    {
        if (!EventLog.SourceExists(source))
            EventLog.CreateEventSource(source, log);

        EventLog.WriteEntry(source, message, type, eventid);
    }

A colleague of mine asked, "why didn't you just create a new class for your event log writer?" So my question is, why would I? And what would this class even look like? And why would I need it when my function works nicely? ok that's 3 questions but you get the point :)

If your app is so small that you have "one and only class and call it when I need it" then keep it inside your class but it is rather rare to require just one class. How big is this class? — Mark Robinson, Aug 19 '11 at 15:27
Creating an event source requires administrative priveleges. It should be done during installation, not dynamically at runtime. — Greg D, Aug 19 '11 at 15:35
This app is a plugin that gets loaded by an app running as the local system account so no issues with security. — Jimmy D, Aug 19 '11 at 17:19

score 1 · Answer 1 · answered Aug 19 '11 at 15:32

why would I?

To encapsulate the logging functionality into its own class. Why? Single Responsibility Principle http://en.wikipedia.org/wiki/Single_responsibility_principle. Ny mixing it into your class you are making that class be responsible for at least two (2) things: whatever it does and logging.

And what would this class even look like?

public class LogWriter
{
    public static Log(string log, string source, string message, EventLogEntryType type, int eventid)
    {
        if (!EventLog.SourceExists(source))
            EventLog.CreateEventSource(source, log);

            EventLog.WriteEntry(source, message, type, eventid);
    }
}

And why would I need it when my function works nicely?

Think about when you are no longer responsible for the code. Think ahead to when the code grows. Eventually, in addition to logging it might have a host of other very helpful functions included in it. The next programmer would be much happier not having to refactor your work because the design precedent has been set.

score 0 · Answer 2 · answered Aug 19 '11 at 15:25

Simple, what if you wish to use this method every where in all other parts of your code base? You again copy - paste. Instead have a helper or a Add in class, where just instantiate and keep calling.

Plus if its in a class, you can have more properties and provide more customization methods as well in logging data.

See if you can make use of built in eventlog/trace stuffs.

score 0 · Answer 3 · edited May 23 '17 at 09:59

I agree that you shouldn't create a new class for writing directly to the event log, but for another reason. That class already exists!

Consider using the built-in debug/tracing mechanisms in System.Diagnostics:

Debug output

Trace output

These are standard classes that dump information to a collection of TraceListener objects, of which many useful types already exist:

DefaultTraceListener - Dumps output to standard debug out, I believe via OutputDebugString().

EventLogTraceListener - Dumps output to the windows event log.

So this changes your output mechanism from a programmatic question into a configuration question. (Yes, if you're working in a straight-up managed app, you can populate your TraceListener collection via your app.config.) That means that everywhere you simply use the appropriate Trace.Write() or Debug.Write() call (Depending on if your want the output in a release build), and the configuration determines where the output goes.

Of course, you can also populate your TraceListener collection programmatically, it's fun and simple.

And this way you don't have to build up your own home-grown logging infrastructure. It's all built-in! Use it in good health! :D

If, on the other hand, you insist on rolling your own (a bad idea, I think), your colleague is right. It's a separate responsibility and belongs in a separate class. I would expect static methods for output because there's probably no concept instances of your debug log. In fact, I'd expect an interface very similar to System.Diagnostics.Debug, so yeah, just use that one instead.

Depending on your approach, you may run into a subtle gotcha' that's in the docs, but not immediately obvious without a careful reading. I found an answer for it elsewhere.

score 0 · Answer 4 · answered Aug 19 '11 at 15:26

If it's a small application (which with one class it must be) then it probably doesn't matter.

But design wise in a larger application, you probably would want to consider having the logging functionality in a class by itself in order to keep each class as narrowly focused as possible.

score 0 · Accepted Answer · answered Aug 19 '11 at 15:29

This is a very general question about OO design. Your colleague is referring to separation of responsibilities; he doesn't think that the idea of an event log writer fits into the abstraction of the class you put it in, and it deserves its own.

If this is all you are ever going to use it for (this one method) and this program is simple enough that you are implementing it one class, there is no need to use another class to interact with your event writer. If you can conceive that your event writer might be used in a different way, or by a different class, in the future, then yes, absolutely make it is own class so that you avoid future problems where you have to change the source code that uses it.

The function you've written is a small function that doesn't keep state, so another class is not really necessary unless it's to avoid future problems.

score 0 · Answer 6 · answered Aug 19 '11 at 15:29

For the same reason that someone put SourceExists(Source) and CreateEventSource(source, log) into their own class, so you could call them just by referencing the assembly that has that class defined, and writing

 EventLog.SourceExists(source);

or

 EventLog.CreateEventSource(source, log);

So if you will never ever need to write to the event log in any other application you ever write, then what you are doing is fine... but if you might ever need this again, then .....

score 0 · Answer 7 · answered Aug 19 '11 at 15:32

I think you should have seperate class because if you are going to create more no.of classes in your application you can use same logging for all of them see below example public static class Logger { private static string logFilePath = string.Empty;

    public static void Log(string logMessage, TextWriter w)
    {
        w.Write( logMessage);
        w.Flush();
    }


    public static void Log(string textLog)
    {
        string directoryString =
      filepath+           @"\Logging";
        Directory.CreateDirectory(directoryString);

        logFilePath = directoryString + "\\" +
          DateTime.Now.ToShortDateString().Replace("/", "") + ".txt";


        StreamWriter sw = null;
        if (!File.Exists(logFilePath))
        {
            try
            {
                sw = File.CreateText(logFilePath);
            }
            finally
            {
                if (sw != null) sw.Dispose();
            }
        }
        using (StreamWriter w = File.AppendText(logFilePath))
        {
            Log(textLog, w);
            w.Close();
        }
    }

Function? Class?

7 Answers7