Adding object to database, using linq or sql connection

Question

I have a table called tblDelegates in my database which I need to populate with some data. I have created a class called Delegates which has various properties to hold the bulk of the information.

I am fairly new to C#, so I wanted to get the answers to a few questions as to why you should be doing things in a particular way and also to get some advice on how I could improve my code.

Please can someone take a look at the questions below and give me a few pointers as to whether this is the correct way to do things.

1. Is this the correct way to do this, by first creating an instance of an object and then adding the fields to it before adding the object to the database?
2. This will sound like a total noob question (so I apologies), but why bother having to create the delegates class object, why could/should you not just add the fields directly into the database?
3. In my Delegates object, should I also add the fields such as Created_User Created_Dttm Updated_User Updated_Dttm, or am I ok leaving them out of this object like I have done below.
4. Is the most efficient/quickest way to do this using LINQ like I have done below, or should I be using a SQL connection, or is it just preference?
5. Can anyone tell me if the formatting of my code is correct, or if I should separate out the steps into separate functions perhaps?

Code to add data to database

        var del = new Delegates
        {
            Pending = true,
            ManagerAlias = row.Cells[4].Text,
            ManagerName = row.Cells[1].Text + " " + row.Cells[2].Text,
            DelegateAlias = ADLookup.AD("alias"),
            DelegateName = ADLookup.AD("GivenName") + ADLookup.AD("sn"),
            DelegateEmail = ADLookup.AD("mail")
        };

        var dc = new LinqToSqlDataContext();
        var Del = new tblDelegate
        {
            Pending = del.Pending,
            Manager_Alias = del.ManagerAlias,
            Manager_Name = del.ManagerName,
            Delegate_Alias = del.ManagerAlias,
            Delegate_Name = del.DelegateName,
            Delegate_Email = del.DelegateEmail,
            Created_User = ADLookup.ADCurrentUser(),
            Created_Dttm = DateTime.Now,
            Updated_User = ADLookup.ADCurrentUser(),
            Updated_Dttm = DateTime.Now
        };

        dc.tblDelegates.InsertOnSubmit(Del);
        dc.SubmitChanges();

Delegates Class

public class Delegates
{
    public bool Pending { get; set; }
    public string ManagerAlias {get;set;}
    public string ManagerName {get;set;}
    public string DelegateAlias {get;set;}
    public string DelegateName {get;set;}
    public string DelegateEmail {get;set;}

    public Delegates()
    { 
    }

    public Delegates(bool pending, string manageralias, string managername, string delegatealias, string delegatename, string delegateemail)
    {
        Pending = pending;
        ManagerAlias = manageralias;
        ManagerName = managername;
        DelegateAlias = delegatealias;
        DelegateName = delegatename;
        DelegateEmail = delegateemail;
    }
}

Answer 1

As @ckuhn203 pointed out, the naming is pretty misleading, but it seems like your domain model calls that entity a Delegate. To dissipate ambiguity I'd call it something like DelegateEntity.

One convention in modeling, is to keep types singular, since an entity represents one single row in that tblDelegates table.

Now, in your insert code, the del instance is not needed. Leaves us with this (notice the name changes) - I'd recommend passing in your connection string to the context's constructor, instead of relying on the one in the designer:

    var context = new LinqToSqlDataContext(connectionString);
    var item = new tblDelegate
    {
        Pending = true,
        Manager_Alias = row.Cells[4].Text,
        Manager_Name = row.Cells[1].Text + " " + row.Cells[2].Text,
        Delegate_Alias = ADLookup.AD("alias"),
        Delegate_Name = ADLookup.AD("GivenName") + ADLookup.AD("sn"),
        Delegate_Email = ADLookup.AD("mail"),
        Created_User = ADLookup.ADCurrentUser(),
        Created_Dttm = DateTime.Now,
        Updated_User = ADLookup.ADCurrentUser(),
        Updated_Dttm = DateTime.Now // shouldn't this one be null?
    };

    context.tblDelegates.InsertOnSubmit(item);
    context.SubmitChanges();

A very important thing to keep in mind, is that a data context implements an interface called IDisposable, which means any instance of that type should be properly disposed, either by explicitly calling its .Dispose() method, or (the better way) by wrapping its usage inside a using block:

using (var context = new LinqToSqlDataContext())
{
    var item = new tblDelegate
    {
        Pending = true,
        Manager_Alias = row.Cells[4].Text,
        Manager_Name = row.Cells[1].Text + " " + row.Cells[2].Text,
        Delegate_Alias = ADLookup.AD("alias"),
        Delegate_Name = ADLookup.AD("GivenName") + ADLookup.AD("sn"),
        Delegate_Email = ADLookup.AD("mail"),
        Created_User = ADLookup.ADCurrentUser(),
        Created_Dttm = DateTime.Now,
        Updated_User = ADLookup.ADCurrentUser(),
        Updated_Dttm = DateTime.Now // shouldn't this one be null?
    };

    context.tblDelegates.InsertOnSubmit(item);
    context.SubmitChanges();
}

The context instance only exists within the using scope, and when the code exits that scope, context.Dispose() gets called, implicitly. Make it a habit to always put IDisposable instances in a using block.

---

Other than that, you're not following the naming convention for parameters: alllowercase is hard to read - should be camelCase:

public Delegates(bool pending, string managerAlias, string managerName, string delegateAlias, string delegateName, string delegateEmail)
{
    Pending = pending;
    ManagerAlias = managerAlias;
    ManagerName = managerName;
    DelegateAlias = delegateAlias;
    DelegateName = delegateName;
    DelegateEmail = delegateEmail;
}

But, instead of having a constructor with [potentially] dozens of parameters, you should simply remove all constructors (the parameterless one is there by default), and instantiate a Delegates like this instead:

var delegates = new Delegates { 
                                  Pending = true, 
                                  ManagerAlias = "foo",
                                  ManagerName = "bar",
                                  DelegateAlias = "_foo",
                                  DelegateName = "_bar",
                                  DelegateEmail = "[email protected]"
                              };

This syntax is the object initializer syntax - notice that you don't need to specify the () after new Delegates, because the syntax is calling the type's default constructor and the compiler knows this, so ()'s are redundant. Of course this syntax can't work if your type doesn't have a default, parameterless constructor.

---

If you have a linq-to-sql data context, you probably have a designer with a .dbml too; in there you can specify new, more C#-friendly names for tblDelegates and its columns, LINQ-to-SQL uses the read-only Source property to know which column it's referring to - this means you can have a table called SOME_UGLY_NAME in your SQL backend, and refer to it as SomeMuchBetterName in your C# code!

...this also means tblDelegates can be mapped to DelegateEntity ;)

Answer 2

If I understand your code (and I'm not 100% certain I do), your Delegates class only returns one object (as opposed to a collection or array of objects) and should thus be named Delegate, but as @Mat'sMug pointed out, it conflicts with the System.Delegate namespace. Since your tblDelegates has nothing to do with that name space, you might want to think up something else. I'm drawing a blank though.

del isn't a very descriptive variable name, but given the context, not too hard to figure out.

There's some inconsistent whitespace here.

public bool Pending { get; set; }
public string ManagerAlias {get;set;}

But that's pretty nit-picky. I'd say this looks pretty good.