Am I starting down a C# bad practice path, creating individual classes for an activity log?

Question

I have a web app that contains an activity log for actions the users take. There are multiple types of activities that can be logged.

Here's an example:

public static class NewAccountActivity() {
  public static Write(string username) {
    //... do stuff to enter a new account activity into the database ...
  }
}
public static class NewPostActivity() {
  public static Write(string username, long postId, string postTitle) {
    //... do stuff to enter a new post activity entry into the database ...
  }
}

And then go on to create a new class for every single type of activity to log. Each activity has a .Write() method, with a unique signature for each one (as shown in the code example above)

And then in my web app (asp.net mvc based) I use them like this:

public ActionResult NewAccount(Account account) {
  if (Model.IsValid(account)) {
    //... do new account stuff ...

    NewAccountActivity.Write(account.UserName);

    //... redirect to action, or show view ...
  }
}

public ActionResult NewPost(Post post) {
  if (Model.IsValid(post)) {
    //... do new post stuff ...

    NewPostActivity.Write(post.UserName, post.postId, post.Title);

    //... redirect to action, or show view ...
  }
}

Is this a bad idea? Is there a better way? Should these be a bunch of methods jammed into one class?

I started doing this because of an answer to a different question I had on SO.

Is there any particular reason you don't just have an `ActivityLogger` class with `LogNewAccount()`, `LogNewPost()`, etc. methods? — Gabe, Aug 06 '10 at 05:12
No particular reason... I've seen the pattern I started using, and began wondering the same thing. Is there a reason to break up everything into it's own class? — Chaddeus, Aug 06 '10 at 06:25

Pierre-Alain Vigeant · Accepted Answer · 2010-08-06T14:46:46.103

This is my suggestion.

First of all, no static class should represent an activity. The Logger class can be static, or accessible through a DI container.

Create an interface for an activity, namely IActivity that has a single Write or Log method. For each activity where the processing is bigger than a single line of code, create a class that implements the IActivity interface.

Now for all other simple activity log, create a default activity that accept a function lambda.

Example, in this code, I assume that each activity builds a string and return it through the Log function:

public class NewAccountActivity : IActivity
{
    private string userName;

    public NewAccountActivity(string userName)
    {
        this.userName = userName;
    }

    public string Log()
    {
        return this.UserName;
    }
}

public class ActivityEntry : IActivity
{
    private Func<string> action;

    public ActivityEntry(Func<string> action)
    {
        this.action = action;
    }

    public string Log()
    {
        return this.action();
    }
}

Now in your static Logger class, create two functions:

public static class Logger
{
    public static void Write(IActivity activity)
    {
        // Ask activity for its data and write it down to a log file
        WriteToFile(activity.Log());
    }

    public static void Write(Func<string> action)
    {
        Write(new ActivityEntry(action));
    }
}

Then in your code, call your logger class like this:

Logger.Write(new NewAccountActivity(currentUserName));

or if you need to log something else that is simple:

Logger.Write(() => "Hello world");

This last call will create a new instance of ActivityEntry that will log "Hello World".

I updated my question a bit. Notice how different activities have different .Write() signatures (because the info within each activity is different, and will be passed to the .Write method from an MVC action). Could you show a concrete example of using your method? I've never tried to do it your way and I don't quite have it clear in my mind yet. Thanks! — Chaddeus, Aug 06 '10 at 05:06

score 2 · Answer 2 · answered Aug 06 '10 at 01:52

2

How about a logger class that takes an enum as an argument so that it knows what type of "activity" it is logging and how to deal with it?

answered Aug 06 '10 at 01:52

Ed S.

122,712
22
185
265

Am I starting down a C# bad practice path, creating individual classes for an activity log?

2 Answers2