CODEWITHSUNDEEP
c#asp.net-mvcmodel-view-controller
Flame_Phoenix
Flame_Phoenix

Reputation: 17564

Use of Model and View (MVC)

I have an application using c# and MVC5, with RazorEngine.

My application manages requests (orders from clients) and I need to show them in tables. To achieve this I have a Controller (HistoryController), a View (Index) and a Model (MaterialRequestModel) which takes several parameters in the constructor:

 public MaterialRequestModel(MaterialRequest order, Employee aConcernedEmployee, Employee anOrderedbyEmployee, Office anOffice)

In my controller, HistoryController I have the following index method, which gets all requests completed or canceled:

public ActionResult Index()
{
    IQueryable<MaterialRequest> query = DB.MaterialRequest
                .Where(m => m.MaterialStatusId == MatStatus.A9Cancelled || m.MaterialStatusId == MatStatus.A8Complete);

    List<MaterialRequestModel> model= new List<MaterialRequestModel>();

    foreach (MaterialRequest req in query)
    {
        model.Add(new MaterialRequestModel(req, DB.Employees.Find(req.ConcernedEmployeeId), DB.Employees.Find(req.OrderedByEmployeeId), DB.Offices.Find(req.OfficeId)));
    }

    return View(model);
}

Now, I could simply pass the query to the the View, but that is not a good practice and the community (this one!) strongly suggests I use Models to avoid placing logic into the view.

However, I can't help to think that the way I am building my model sucks terribly bad, because I am iterating over a large set of results.

How can I improve this code and make it decent without the loop?

Additionally, should I pass everything as a parameter to my model, or should I just pass the DB object into the Models constructor and have it do the queries there?

Upvotes: 1

Views: 95

Answers (2)

Sam I am says Reinstate Monica
Sam I am says Reinstate Monica

Reputation: 31184

        IQueryable<MaterialRequest> query= DB.MaterialRequest
            .Where(m => m.MaterialStatusId == MatStatus.A9Cancelled || m.MaterialStatusId == MatStatus.A8Complete)
            .Select(m => new MaterialRequestModel(m, DB.Employees.Find(m.ConcernedEmployeeId), DB.Employees.Find(m.OrderedByEmployeeId), DB.Offices.Find(m.OfficeId))
            .ToList();

If you just don't want to see the loop, you can use a select statement. and if this is EF, you have the added benefit of not querying your DB in the loop, since it will translate that select statement into sql.

Also, if you want to go to the next level, you can use foreign key references instead of all the DB.Employees.Find(...) Here's the result of my googling for that

Upvotes: 4

Oliver
Oliver

Reputation: 1245

I try to answer some of your questions :)

eliminating the loop:

Create a query like Sam I am suggested. This way you would get all the data with single query and you could eliminate the loop.

the model:

Personally I like POCOs a lot, because I looks cleaner to me. Thats why I would not pass the DB into my model.

Upvotes: 1

Related Questions