3

I need to perform some kind of auditing. We want to store when a record is inserted, updated, removed or opened.

For now I have created a simple method on a Singleton class:

public void Audit(string audit, AuditTypes type)
{
    AuditEntry = new AuditEntry(){ Audit = audit, TypeId = (int)type };

    // some logic to commit the audit entry to the database
}

public enum AuditTypes
{
  Insert = 1,
  Update = 2,
  Delete = 3
  Open = 4
}

Somewhere in the forms I call this method:

MyForm.cs:

private void RemoveSomeObject(SomeObject myObject)
{
   /* Do some stuff that removes the object*/

   MySingleton.GetInstance().Audit(myObject.Title, AuditTypes.Delete)
}

For some reason, I don't think this is the way to go because using this approach everywhere in the code I have this kind of lines.

I think it is smarter to do it a more OO way, what do you think? EDIT:

I do log the User id and date, but I didn't find it relevant to notice.

Martijn
  • 24,441
  • 60
  • 174
  • 261
  • 1
    I don't know if this is an option for you, but in the past I've worked on systems that do auditing with insert/update/delete triggers against a *table*_audit version of the table. – asawyer Feb 03 '12 at 13:41
  • 1
    Which RDBMS? Auditing data changes *really* belongs on the database... – Yuck Feb 03 '12 at 13:42
  • He hasn't even mentioned a database, why assume there is a database involved? It could be a Twitter spambot for all we know. – Aidan Feb 03 '12 at 13:46
  • While you can certainly audit which changes happen at the database level, it's more cumbersome to audit who did the changes when doing it at the database level. When you have a web app, most of the time, all users of the web app access the database under a single user account, but what you really want to track is which user at the application level (not the database) made the changes. Unless the only way to access the database is through stored procedures which require the application user credentials to be passed in, this is something that can only be done at the application level. – Kibbee Feb 03 '12 at 13:47
  • @Yuck We are using MS SQL Server 2005 and higher – Martijn Feb 03 '12 at 13:52
  • Your audit does not currently include who performed the action and when. If string and type is all you are tracking then I agree do it in the database. If the user does not connect directly to the db (good application design) and you want to track the changes to the user then audit does need to initiate in the app to get unique user ID (e.g. login). Then I do audit on the set of the properties and I do it directly in the set as the set has to open a connection anyway. You can even wrap the audit in a transaction if you want to be sure of no change without audit. – paparazzo Feb 03 '12 at 13:58
  • @BalamBalam What do you mean with 'then I agree do it in the database' – Martijn Feb 03 '12 at 14:27
  • See the first comment - triggers. – paparazzo Feb 03 '12 at 21:54

4 Answers4

2

When preforming CRUD type actions it is often good to encapsulate the data access layer using a Repository Design Pattern. You could have a base class for your Repository classes that handles the auditing for you.

Kevin Junghans
  • 17,475
  • 4
  • 45
  • 62
2

This is a clasic example of Aspect-oriented programming. Basically you have a crosscutting requirement that will spread to many parts of the system (i.e. logging or auditing). The problem is that your approach seems correct, but could be hard to maintain and doesn't scale well. If you have the time and want to, you can read about this and give it a try using PostSharp, they have a free starter edition. You can also check this: AOP in .NET

Diego
  • 1,531
  • 1
  • 15
  • 27
1

Well you should certainly avoid the singleton (google for its inherent evilness), but I certainly don't think there is much else wrong with your approach. And it's only smarter to do something in an OO way or otherwise if it makes you code better against some factor that you find important like readability or performance or correctness. OO-ness is not really that important.

So to get round your singleton I would inject an instance of your Auditor whenever you do an operation that needs auditing :

private void RemoveSomeObject(SomeObject myObject, Auditer myAuditer)
{
    // do stuff //

    myAuditer.Audit(...);
}

(By the way you should probably take the remove logic out of your form as well, and put it in another layer - each class should only have one responsibility)

Aidan
  • 4,783
  • 5
  • 34
  • 58
  • By using this approach, the caller must know that the action is going to be audited. Does this makes sense? – Martijn Feb 03 '12 at 13:53
  • If the RemoveSomeObject method is moved in the domain layer (as I suggest), the yes. The user would just call a Remove() method which then calls into the RemoveSomeObject() method. The user won't know about the Auditer, but the domain layer will. – Aidan Feb 03 '12 at 14:13
  • If you're programming against an interface, you can follow the Decorator Pattern to add auditing as needed. – Christopher Stevenson Apr 03 '13 at 19:00
0

I would suggest doing the auditing at the lowest level possible within your data layer to reduce the number of method calls to your auditer object.

From my experience with auditing, I've found it useful to keep a copy of the before and after images, the date and time of the change and who made the change.

TeamWild
  • 2,460
  • 8
  • 43
  • 53