StackOverflow2013

Note that there are some explanatory texts on larger screens.

plurals

PO
text
Body
copied!<p>There are a few improvements to be made.</p> <p><strong>Save() Method - Don't left-to-right copy, use EF built in logic</strong></p> <p>Instead of this:</p> <pre><code>myEmp.Employee_ID = emp.Employee_ID; myEmp.First_Name = emp.First_Name; myEmp.Middle_Name = emp.Middle_Name; myEmp.Last_Name = emp.Last_Name; myEmp.Supervisor_ID = emp.Supervisor_ID; myEmp.Active = emp.Active; myEmp.Is_Supervisor = emp.Is_Supervisor; </code></pre> <p>You can do this:</p> <p><code>ctx.Employees.ApplyCurrentValues(emp)</code>.</p> <p>What this does, is look for an entity with the same key in the graph (which there is, since you have just retrieved it with <code>FirstOrDefault()</code>), and override the scalar values with the entity you pass in - which is exactly what your doing.</p> <p>So your 7 lines becomes 1, plus if you add any extra scalar properties - you won't have to refactor your code. Just remember - only works for <em>scalar properties</em>, not <em>navigational properties</em>.</p> <p><strong>Why build query for primary key retrieval? Just use predicate to SingleOrDefault()</strong></p> <p>Instead of this:</p> <pre><code>var results = from item in ctx.Employees where item.ID == emp.ID select item; var myEmp = results.FirstOrDefault(); </code></pre> <p>Do this:</p> <pre><code>var myEmp = ctx.Employees.SingleOrDefault(x => x.ID == emp.Id); </code></pre> <p>Or even better, use a <strong>pipe/filter</strong> technique:</p> <pre><code>var myEmp = ctx.Employees.WithId(emp.Id).SingleOrDefault(); </code></pre> <p>Where <code>WithId</code> is an <code>IQueryable<Employee></code> extension method, which filters the query based on the supplied employee ID. This allows de-coupling of filtering/business logic from your repository/DAL. It should go in your domain model, so you can have a nice fluent API for query your domain entities via your ORM.</p> <p>When your retrieving an entity via the primary key, you should <strong>always</strong> use <code>SingleOrDefault()</code> or <code>Single()</code>, never <code>FirstOrDefault()</code> or <code>First()</code>. If it's a primary key - there should only be one of them, so you should throw an exception if more than one exists, which is what <code>SingleOrDefault()</code> does. And as @Shiraz mentions - your <code>FirstOrDefault()</code> will crash the query below. You always need null checking when you use <code><First/Single>OrDefault()</code>.</p> <p>The same improvements can be made to your Get method.</p> <p><strong>Overall,</strong> there is nothing functionally wrong with your code - it just needs subtle improvements, null checking and exception handling.</p> <p><strong>The only functional improvement i highly recommend</strong> is refacting your web service code into a Generic Repository. As the code is very trivial and can be re-used across any entity. The web service shouldn't be concerned with transactions, primary key's or EF logic whatsoever. It shouldn't even have a reference to the EF DLL. Encapsulate this logic behind a repository and delegate the persistence logic to there (via an interface of course).</p> <p>After making the changes i've mentioned above, your web service methods should have no more than 5-7 lines of code each.</p> <p>You have far too much intelligence in your web service - it should be dumb and persistent ignorant.</p>

Querying!

Guidance

An individual column

Larger individual text columns get their own page to allow for proper reading.

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload