Building a better Data Access Layer Class

Since I am new to MVC and the Entity Framework, I have been struggling to grasp the concept of creating useful Data and Service layers. I have come across a scenario where I believe my code has become very redundant:

Data Access Layer:

public static class DeploymentOrderDataLayer
    {

     public static usr_OrderFulfillment GetScannedItem(string orderNumber, string itemNumber)
        {
            using (TPGContext context = new TPGContext())
            {
                return context.usr_OrderFulfillment.FirstOrDefault(x => x.SOPNUMBE == orderNumber && x.ItemNumber == itemNumber);
            }
        }

    public static void UpdateScannedItem(string orderNumber, string itemNumber, string user, string serialNumber)
        {
            using (TPGContext context = new TPGContext())
            {
                var result = context.usr_OrderFulfillment.FirstOrDefault(x => x.SOPNUMBE == orderNumber && x.ItemNumber == itemNumber);

                result.IsFulfilled = true;
                result.FulfilledBy = user;
                result.DateFulfilled = DateTime.Now;

                if (!string.IsNullOrEmpty(serialNumber))
                {
                    result.SerialNumber = serialNumber;
                }

                context.SaveChanges();
            }
        }
}

I call all of my Data Layers through a BusinessLayer, which handles my business logic:

```
public class DeploymentOrderBusinessLayer
{

public static void UpdateScannedItem(string orderNumber, string itemNumber, string user, string serialNumber = "")
{

var itemInfo = DeploymentOrderDataLayer.GetScannedItem(orderNumber, itemNumber);

if (itemInfo == null) throw new BusinessException("Item number not found.");

if (itemInfo.IsFulfilled) throw new BusinessException(string.Format("Item already fulfilled by {0}", itemInfo.FulfilledBy));

DeploymentOrderDataLayer.UpdateScannedItem

This is going to be just a partial review, I don't have much time but I can't help it, I have to say something.

-
usr_OrderFulfillment might be how the table is called in your database, and SOPNUMBE might be how a column is called in that table. It doesn't mean your entities have to be as painful. Entity Framework is an object/relational mapper; this implies a mapping - you can easily map table usr_OrderFulfillment to entity OrderFulfillment and column SOPNUMBE to property OrderNumber.

-
Your DeploymentOrderDataLayer class (I wouldn't call a class a layer - call it a service if you will, but a layer is more for an assembly than for a type), is tightly coupled to TPGContext, and each new method requires a new instance of your context - even if 20 calls are made in the same request. You should inject the instance through the class' constructor and use a single instance per request. There's really no need to new it up and dispose it in every method. The class being static isn't helping either (it does explain the non-existing constructor though). Always prefer instances over static helpers.

-
This architecture feels like stored procedures written in C# code. It's bloated, over-engineered and makes you almost as good as me in shredding KISS into tiny little pieces. Have you tried using the EF DbContext directly in your controller to get the job done, and then refactor into a more structured approach?

StackExchange Code Review Q#38843, answer score: 5