Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've used EF-DB first approach and have written a simple app to retrieve the data from the database and show it in the view. The code is simple and ok for a beginner like me, but how can I implement an effective design-pattern like interface-specification pattern or generic repository pattern, and possibly dependency injection to improve the code below ? I'd like to know because when extending the project I might have to get data from different databases, and would like to implement an effective pattern for maintainability.

Please note that I've started looking into topics like interfaces, design patterns, etc only recently, So I'd greatly appreciate it if you could please break down the explanation to a beginner level.

Below are my view-model and the controller. My model contains a stored-procedure and a table in the .edmx file - Thanks

View-Model

    public class OfficeDTO
    {
        public IEnumerable<EmployeeDTO> Employees { get; set; }
        public IEnumerable<DepartmentDTO> Departments { get; set; }
    }

    public class EmployeeDTO
    {
       public int EmpId { get; set; }
       public string EmpName { get; set; }
    }

    public class DepartmentDTO
    {
        public string DeptCode { get; set; }
        public string DeptName { get; set; }
    }

Controller

  public ActionResult Index()
            {
                EF.OfficeEntities ctx = new EF.OfficeEntities();
                Models.OfficeDTO office = new Models.OfficeDTO();

                using (ctx)
                {

                    var empList = ctx.GetEmployeesByYearJoined("2009")
                              .ToList();

                    var empResults = (from q in empList
                                   select new Models.EmployeeDTO
                                   {
                                       EmpId = q.EmpID,
                                       EmpName = q.FirstName + q.LastName
                                   }).ToList();
                    office.Employees = empResults;

                    var depResults = (from q in ctx.Departments
                                      select new Models.DepartmentDTO
                                      {
                                          DeptCode = q.DepartmentCode,
                                          DeptName = q.DepartmentName

                                      }).ToList();
                    office.Departments = depResults;
                }

                return View(office);
            }
share|improve this question
1  
If you want to improve performance, you can start by removing the intermediate ToList() calls, you probably don't even need em. If EF complains about the operations you're doing and you're not doing any more filtering beyond that, use AsEnumerable() instead to make it a LINQ to Objects query. –  Jeff Mercado Feb 18 '13 at 9:25
    
@JeffMercado In the current implementation, if the .ToList() calls are removed then there will be an error as the context is disposed before the view is rendered. –  Trevor Pilley Feb 18 '13 at 10:42
    
@TrevorPilley: Not between empList and empResults, the ToList() call there is superfluous. –  Jeff Mercado Feb 18 '13 at 15:01
    
@JeffMercado yes, the one on var empList... is unnecessary overhead the other two calls however are still needed. –  Trevor Pilley Feb 18 '13 at 22:22
    
I need the empList because I want to concatenate the firstname and lastname in empResults. do you think it'll still work if i make empList AsEnumerable() ? –  user1980311 Feb 20 '13 at 16:29

1 Answer 1

up vote 1 down vote accepted

The only way DI (IoC) is going to improve this code is by extracting the creation and disposal of your data context. This also means you can mock it for your unit tests.

You could argue the DataContext already is a Repository.

You could do one of:

  • Leave it as it is. No reason to complicate it just for the sake of it. It's small, pretty easy to follow and easily found.
  • Extract a Query object (Query Pattern)
  • Wrap the query and creation of the OfficeDTO in an OfficeDTO factor
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.