views:

511

answers:

4

I'm wondering if it's bad practice to use log4net directly on my domain object... I'll be using ELMAH for my exceptions on the ASP.NET MVC application side, but for some informational purposes I'd like to log some data about the domain model itself.

Given the following domain object:

   public class Buyer
{
    private int _ID;
    public int ID
    {
        get { return _ID; }
        set
        {
            _ID = value;
        }
    }

    private IList<SupportTicket> _SupportTickets=new List<SupportTicket>();
    public IList<SupportTicket> SupportTickets
    {
        get
        {
            return _SupportTickets.ToList<SupportTicket>().AsReadOnly();
        }
    }

    public void AddSupportTicket(SupportTicket ticket)
    {
        if (!SupportTickets.Contains(ticket))
        {
            _SupportTickets.Add(ticket);
        }
    }
}

Is adding logging behavior in the AddSupportTicketMethod a bad idea...so essentialy it'd look like this:

       public class Buyer
{
    protected static readonly ILog log = LogManager.GetLogger(typeof(SupportTicket));

    public Buyer()
    {
       log4net.Config.XmlConfigurator.Configure();
    }


    private int _ID;
    public int ID
    {
        get { return _ID; }
        set
        {
            _ID = value;
        }
    }

    private IList<SupportTicket> _SupportTickets=new List<SupportTicket>();
    public IList<SupportTicket> SupportTickets
    {
        get
        {
            return _SupportTickets.ToList<SupportTicket>().AsReadOnly();
        }
    }

    public void AddSupportTicket(SupportTicket ticket)
    {
        if (!SupportTickets.Contains(ticket))
        {
            _SupportTickets.Add(ticket);
        } else {
           log.Warn("Duplicate Ticket Not Added.");
        }
    }
}
+1  A: 

This is a classic question!

The good way of doing this would be to introduce a class member of ILogger type and abstract the logging into this interface. In your class wherever you do a call to logg something do it through this interface. Then inject this dependency at the run-time with one of the implementation using one of the available IoC container or dependency injection farmeworks. By default you can use log4net implementation of this interface.

Here is a long list of available dependency injection frameworks: http://www.hanselman.com/blog/ListOfNETDependencyInjectionContainersIOC.aspx

bychkov
A: 

I think logging is a cross cutting concern, so it's best done in an aspect-oriented fashion. If you're using a framework like Spring.NET it's available to you.

duffymo
+1  A: 

I have used log4net and log4J directly in domain objects. This has good side effects and bad ones.

  • +: Logging in the domain object is simple and straightforward to code and you know you can take advantage of log4net features.
  • --: It means the program making use of the domain objects needs to pay attention to log4net configuration, which may or may not be a problem
  • --: You cannot link your domain object to a different log4net version than the calling program is using. I've seen a lot of conflicts with one item linked against log4net 1.2.0.10 and another linked against an earlier release.

Not logging in your domain object is a bad idea. The alternative is as others have suggested, dependency injection or an external framework (such as commons-logging for log4J) that allows plugging different logging frameworks or creating an interface that does the logging and logging against that interface. (The code using your domain object would need to then supply an appropriate instance of that interface for logging purposes.)

Eddie
+2  A: 

If you are going to log from your domain objects and you use an IOC container which you might want to swap out, I would recommend you use the Service Locator pattern (you could look at the Sharp# architecture for a nice implementation of a SafeServiceLocator that wraps msoft's ServiceLocator with more informative error messages).

I would also like to suggest that you consider whether you want to log the type of error you show in your example. I would tend to want to have the domain object throw an exception in that case and let the caller decide whether that was something that was expected by the application (and hence shouldn't be logged) or whether that represents a situation that the caller wants to deal with in some way.

Rob Scott