Any way I can make this code cleaner/more efficient? (C#/LINQ-to-SQL)

Question

I've been putting together an auditing solution for a program I am developing, where I am using LINQ for my update/insert operations. I have come up with the following solution (this is the snippet for inserting) (Note the Tables variable contains a list of all the tables that have been modified - I add these to the list manually and call this method):

 BindingFlags b = BindingFlags.Instance | BindingFlags.Public;
        LINQDataContext dc = new LINQDataContext();
        foreach (object Table in Tables)
        {
            string TableName = Table.ToString().Replace("Project.", "");

            switch (TableName)
            {
                case "Job":
                    string NewJobString = null;
                    Job JobDetails = (Job)Table;
                    var prpsJob = typeof(Job).GetProperties(b);
                    foreach (var p in prpsJob)
                    {
                        object x = p.GetGetMethod().Invoke(JobDetails, null);
                        x = StripDate(x);
                        NewJobString += p.Name + ": " + x + Environment.NewLine;
                    }
                    Audit(JobID, NewJobString, "New Job created", SourceID, "", JobDetails.JobID);
                    break;

                case "Estimation":
                    string NewEstimationsString = null;
                    Estimation EstimationDetails = (Estimation)Table;
                    var prpsEstimations = typeof(Estimation).GetProperties(b);
                    foreach (var p in prpsEstimations)
                    {
                        object x = p.GetGetMethod().Invoke(EstimationDetails, null);
                        x = StripDate(x);
                        NewEstimationsString += p.Name + ": " + x + Environment.NewLine;
                    }
                    Audit(JobID, NewEstimationsString, "New Estimation created", SourceID, "", EstimationDetails.EstimationID);
                    break;

And the code goes on for each possible tablename. The code works fine, but it seems fairly inefficient - having a nearly identical block for each case. Is there a more efficient way?

PLINQO have audit functionality in the DataContext: http://www.codesmithtools.com/product/frameworks audit details: http://www.codesmithtools.com/product/frameworks/plinqo/tour/auditing — Guillaume86, Mar 07 '11 at 14:41

score 2 · Accepted Answer · answered Mar 07 '11 at 17:11

You should be able to use Lambdas to cover the type-specific parts of the repeated code. This is some almost pseudo-code I hacked together....

void TableIsJob(Job j, BindingFlags b) {
   HandleTable("Job", j.JobID, typeof(Job).GetProperties(b),
               p=>p.GetGetMethod().Invoke(j, null));
}


void TableIsEstimation(Estimation e, BindingFlags b) {
   HandleTable("Estimation", e.EstimationID, typeof(Estimation).GetProperties(b),
       p => p.GetGetMethod().Invoke(e, null));
}

void HandleTable(string nm, int ID, PropertyInfo [] props, Func<PropertyInf, Object> i) {
       string desc = string.Join(Environment.NewLine, props.Select(p=>{
                       return string.Format("{0}: {1}", p.Name,
                                    StripDate(i(p)));
               }).ToArray());
       Audit(JobID, desc, string.Format("New {0} created", nm),
             SourceID, "", id);
}

And then you can replace your huge for loop and switch case with...

Tables.Select(t =>
{
   switch (t.ToString().Replace("Project.", ""))
   {
       case "Job":
           TableIsJob((Job)t, b);
           break;
       case "Estimation":
           TableIsEstimation((Estimation)t, b);
           break;
   }
});

This is all assuming that "efficient" means in terms of code volume, not in execution time.

The_Butcher · Answer 2 · 2011-03-07T14:41:49.027

Well you can certainly pull out the below code into a method:

 string NewJobString = null;
    Job JobDetails = (Job)Table;
    var prpsJob = typeof(Job).GetProperties(b);
    foreach (var p in prpsJob)
    {
           object x = p.GetGetMethod().Invoke(Table, null);
           x = StripDate(x);
           NewJobString += p.Name + ": " + x + Environment.NewLine;
    }

public string GetProperties(Type t, BindingFlags b)
{
     StringBuilder sb = new StringBuilder();
     var prpsJob = typeof(t).GetProperties(b);
     foreach (var p in prpsJob)
     {
          object x = p.GetMethod().Invoke(Table, null);
          x = StripDate(x);
          sb.Append(p.Name + ": " + x + Environment.NewLine);
     }
     return sb.ToString()
}

And have something like

 case "Job":

   Audit(JobID, GetProperties(typeof(Job),b), "New Job created", SourceID, "", JobDetails.JobID);
   break;

Thorarin · Answer 3 · 2011-03-07T14:50:34.250

From what I can gather, most of the duplicated code is dumping the value of the objects into a string.

You could use a helper method to do this:

public static string DumpObject<T>(T obj)
{
    StringBuilder sb = new StringBuilder();

    var properties = typeof(T).GetProperties(
                     BindingFlags.Instance | BindingFlags.Public);

    foreach (var p in properties)
    {
        object x = p.GetGetMethod().Invoke(obj, null);
        x = StripDate(x);
        sb.Append(p.Name).Append(": ").Append(x).AppendLine();
    }

    return sb.ToString();
}

This would still not be very efficient code in terms of execution time, but it would reduce the code duplication. I've also changed the string appending to using a StringBuilder. It's just good practice in cases like this.

By the way, it is convention to use camel case for local variables and parameters:

switch (tableName)
{
    case "Job":
        Job jobDetails = (Job)table;
        Audit(jobID, DumpObject(jobDetails), "New Job created",
              sourceID, "", jobDetails.JobID);
        break;
    // ...
 }

Jaben · Answer 4 · 2011-03-07T15:16:45.220

I suggest you use T4 templates to generate your LINQ to SQL classes by using LINQ to SQL templates for T4. Then add an interface on the tables that have special needs.

public interface IHaveEstimation {
   DateTime GetEstimationDate();
}

In your LINQDataContext.tt file, you can add some additional T4 generation code that detects the table and add the interface to that Object:

<#  
  string markerInterface = String.Empty;

  if (class1.Name == "Estimation")
  {
     markerInterface = "IHaveEstimation"; 
  }
#>

<#=code.Format(class1.TypeAttributes)#>partial class <#=class1.Name#>
<#=String.IsNullOrEmpty(markerInterface) ? String.Empty : String.Format(" : {0}", markerInterface) #>
{ ... }

In your LINQDataContext.cs file, you can do something like this:

/// <summary>
/// When the database context is submitted.
/// </summary>
/// <param name="failureMode">
/// the submit failure mode
/// </param>
public override void SubmitChanges(ConflictMode failureMode)
{
  foreach (var insert in changeSet.Inserts.OfType<IHaveEstimation>())
  {
    var estimtation = insert.GetEstimationDate();
    // handle auditing, etc.
  }

  // do same for update and delete change sets
}

score 0 · Answer 5 · edited May 23 '17 at 09:58

0

You should really use this answers from SO quesetion

And In case You do not want to serialize all properties I STRONGLY advise creating a custom attribute and filtering the list based on it (of course decorating the properties before). Messing with IL is dirty at the beggining but it is THE most performant way to do it. Hand's down.

luke

edited May 23 '17 at 09:58

Community

1
1

answered Mar 07 '11 at 14:57

luckyluke

1,553
9
16

Any way I can make this code cleaner/more efficient? (C#/LINQ-to-SQL)

5 Answers5