Using a separate class file (model) for Entity Framework queries instead of writing in controller itself

Question

Is it ok to write EF queries in a separate class file inside models folder instead of writing it in controller itself.

Because I've seen in msdn writing all EF queries in controller itself. While at the same time I have also read in msdn once that controller is meant to be short.

Using models I use this approach:

In the controller:

        public JsonResult SaveStorageLocation(StorageLocations objStorageLocation)
        {
            int Result = 0;
            try
            {
                StorageLocationModel objStorageLocationModel = new StorageLocationModel();
                if (objStorageLocation.Id == Guid.Empty)
                {
                    Result = objStorageLocationModel.AddStorageLocation(objStorageLocation, SessionUserId);
                }
                else
                {
                    Result = objStorageLocationModel.UpdateStorageLocation(objStorageLocation, SessionUserId);
                }
            }
            catch (Exception ex)
            {
                Result = (int)MethodStatus.Error;
            }
            return Json(Result, JsonRequestBehavior.AllowGet);
        }

In Model class:

public int AddStorageLocation(StorageLocations objStorageLocation, Guid CreatedBy)
        {
            MethodStatus Result = MethodStatus.None;
            int DuplicateRecordCount = db.StorageLocations.Where(x => x.Location.Trim().ToLower() == objStorageLocation.Location.Trim().ToLower()).Count();
            if (DuplicateRecordCount == 0)
            {
                objStorageLocation.Id = Guid.NewGuid();
                objStorageLocation.CreatedBy = CreatedBy;
                objStorageLocation.CreatedOn = DateTime.Now;
                objStorageLocation.ModifiedBy = CreatedBy;
                objStorageLocation.ModifiedOn = DateTime.Now;
                objStorageLocation.Status = (int)RecordStatus.Active;
                db.StorageLocations.Add(objStorageLocation);
                db.SaveChanges();
                Result = MethodStatus.Success;
            }
            else
            {
                Result = MethodStatus.MemberDuplicateFound;
            }
            return Convert.ToInt32(Result);
        }

        public int UpdateStorageLocation(StorageLocations objStorageLocationNewDetails, Guid ModifiedBy)
        {
            MethodStatus Result = MethodStatus.None;
            int DuplicateRecordCount = 
                db.StorageLocations.
                Where(x => x.Location == objStorageLocationNewDetails.Location && 
                x.Id != objStorageLocationNewDetails.Id).Count();
            if (DuplicateRecordCount == 0)
            {
                StorageLocations objStorageLocationExistingDetails = db.StorageLocations.Where(x => x.Id == objStorageLocationNewDetails.Id).FirstOrDefault();
                if (objStorageLocationExistingDetails != null)
                {
                    objStorageLocationExistingDetails.Location = objStorageLocationNewDetails.Location;
                    objStorageLocationExistingDetails.ModifiedBy = ModifiedBy;
                    objStorageLocationExistingDetails.ModifiedOn = DateTime.Now;
                    objStorageLocationExistingDetails.Status = (int)RecordStatus.Active;
                    db.SaveChanges();
                    Result = MethodStatus.Success;
                }
            }
            else
            {
                Result = MethodStatus.MemberDuplicateFound;
            }
            return Convert.ToInt32(Result);
        }

Or is it better to write all the code in controller itself?

score 1 · Answer 1 · answered Jan 22 '20 at 06:37

I expect your question will be closed pretty soon because it will be deemed opinion-based.

With that aside, there are many advantages if you don't have your queries in your controller.

Controller should not dictate how to access data, that's the job for model.
It is much easier to mock the data access if you inject the model (or service or repository, whatever you want to call it your application) as a dependency.
You may find out later on that certain queries are much better handled if you migrate them to stored procedures, for they process large amounts of data. This change will be easier to make if controller does not access the data store directly. You could either make the changes in your model class and keep the same interface, or write a new implementation which then gets injected.
Both controller and model can be more easily tested in isolation from each other.

score 1 · Answer 2 · answered Jan 22 '20 at 06:39

You want your code to be testable, always.

Never put logic in your models, putting logical items in the model-folder will make your structure dirty and it’s easier to loose overview.

You should use a repository class, that implements an interface. In the repository class you can perform EF logic, catch database exceptions and so on.

Your controller will be injected with the repository interface. This way you can test your controller separately from the EF logic, because you can mock that interface. Your repository will be testable for it’s oen functionality as well.

Using a separate class file (model) for Entity Framework queries instead of writing in controller itself

2 Answers2