C# Discussion Boards - CodeProject

1-Feb-14 10:13

I'm a freelance programmer, and one of the major bugbears with working by myself is that I have no-one to bounce ideas off, so my code becomes very personal, if that's the right word, and I sometimes wonder if I'm doing things the right way. For instance, when I interrogate my database using Linq to Entities I use the following code (to return an instance of the purchase class).

internal static Purchase ReturnPurchaseItem(int PiD)
{
RegulusEntities RG = new RegulusEntities();
Purchase purchase = (from PU in RG.Purchases
where PU.PurchaseID == PiD
select PU).FirstOrDefault();
return purchase;
}

I would be very grateful if you could critique my code and maybe suggest more efficient ways of returning an instance of a class, or using the entity framework for that matter.
(If I've got this in the wrong forum, please forgive me, and transfer me to the right one!).

Dave Kreskowiak1-Feb-14 13:07

1-Feb-14 13:07

There simply isn't enough code here to make any real judgment of what you're doing. I can see some smallish things but nothing about any patterns you're using.

1) Your method is called "Return..." something. Personally, I always name methods that return something "Get...". Return is a bit more vague as to what it does. Does "Return" refer to retrieving a database object, or does it refer to some business rule method. It's the concept of "self documenting code". Method names should describe exactly what they do.

2) Entities (or DbContext) classes should be Disposed when you're done with them. It should be something like this:

internal static Purchase GetPurchaseItem(int purchaseId)
{
    Purchase purchase = null;

    using (var context = new RegulusEntities())
    {
        return context.Purchases.Find(purchaseId);
    }
}

3) If you have a primary key Id for an object, you don't need an entire Linq query to find it. You just need to call the Find method to retrieve it. Find() will return the object with the specified key. If it's not found, it'll throw a InvalidOperationException. You can replace it with FirstOrDefault() if you want the "Get..." method to return a null of the specified type instead of throwing.

Other than that, a couple of rules I follow:

I don't have my database code return any ViewModel objects, not does it accept any. My database layer will accept and return Entity objects only. If these objects need to be translated into ViewModel objects, I can have another layer above this that does the translation, such as in an MVC Controller class or what I call a "Services" class.

I never expose my database layer directly to UI code.

A guide to posting questions on CodeProject[^]

Dave Kreskowiak

Message Closed

2-Feb-14 9:51

cseder

2-Feb-14 9:51

Message Closed

Dave Kreskowiak2-Feb-14 11:46

2-Feb-14 11:46

You might want to be careful whom you reply to. Now that you replied to the wrong person, the OP will not get an email saying that someone else replied to him.

A guide to posting questions on CodeProject[^]

Dave Kreskowiak

cseder2-Feb-14 15:19

cseder

2-Feb-14 15:19

My eyes just played a trick on me.

I've corrected the reply to the right person now...

Certainty of death, small chance of success...
What are we waiting for?

Shady George2-Feb-14 15:50

2-Feb-14 15:50

Thanks for the reasoned and well thought out answer Dave. In showing you only a tiny patch of my code, I actually missed out the whole structure that this little method sits in.

This method, as with all my other calls to EF sits in a service class written to handle all the calls to a particular table. for example, all the calls to the purchase table are made from the clsPurchaseHandler class, thus separating it from the UI.

Your answer is exactly what I wanted. as I explained I am a freelance programmer running my own little operation, and I haven't got the luxury of being able to discuss code with anyone, and there's always the niggle at the back of my mind that 'I know it works, but is it the most efficient'. Your code detailing the disposal of entities after use and the find method is invaluable.

As for my naming of the method "ReturnPurchaseItem", that's my usual practise, as that's what the method does, I take your point that it may sound vague, but I've got a lot of code out in the wild that uses this notation, so for now, I'll stick with it.

Thanks again Dave, much appreciated.

George

George

Shady George2-Feb-14 17:05

2-Feb-14 17:05

Sorry to be a pest Dave, but the 'find' method you suggest isn't available to my version of EF. I've read up on it and it appears to come in after EF 4.0. I appear to be running EF 6.0, but I still can't see it. What am I doing wrong?

Dave Kreskowiak2-Feb-14 18:52

2-Feb-14 18:52

You have to import the System.Data.Entity namespace at the top of the code file.

If you get the red squiggly under Find, right click the word and then you should be able to pick Resolve -> System.Data.Entity.

A guide to posting questions on CodeProject[^]

Dave Kreskowiak

Shady George4-Feb-14 8:28

4-Feb-14 8:28

I've done all of that Dave, and I still get the red squiggly. It's got to be something to do with the version of the System.data.entity dll that I'm importing. EF reports that it's using version 6, but the DLL reports version 4. I'll keep trying, but until then, I'll use the version that works, but I can't help feeling that i's going to get up and bite me one day.

Dave Kreskowiak4-Feb-14 10:11