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
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!).
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:
internalstatic Purchase GetPurchaseItem(int purchaseId)
Purchase purchase = null;
using (var context = new RegulusEntities())
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.
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.
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?
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.
If you've got a mismatch like that, something went terribly wrong.
Go to the Package Manager Console and do a Uninstall-Package EntityFramework. Then go through the References in the project and remove the EntityFramework reference if it's still there.
Once that's done, you can go back to the Package Manager Console and do a
Install-Package EntityFramework".<br />
That should take care of any mismatches.<br />
<div class="signature"><small><a href="http://www.codeproject.com/scrapbook/ForumGuidelines.asp">A guide to posting questions on CodeProject</a>[<a href="http://www.codeproject.com/scrapbook/ForumGuidelines.asp" target="_blank" title="New Window">^</a>]</small><br><br />
<font color="Blue">Dave Kreskowiak</font><br />
I've been battling since Tuesday to fix this Dave. (sorry for taking so much of your time), I've done what you detailed above, but still the 'find' method isn't recognised. Funny thing though, the text I get when I try to use the method is:
"Error 1 'System.Data.Objects.ObjectSet<entityresearch.purchase>' does not contain a definition for 'find' EntityResearch...."
I thought the find method would be in System.Data.Entity?
I've gone so far as repairing my .NET 4.5 but that didn't work either.
Repairing anything isn't going to do anything for you. Nothing is broken except your code.
There are multiple Find methods. The one that works on DbSets is in System.Data.Entity. If it's saying the you're using the System.Data.Objects namespace version, you're not using Entity Framework. Your entity sets have been defined using ObjectSet instead of DbSet.