1
\$\begingroup\$

I'm curious about the best way to instantiate my repositories. Currently they're in the main controllers constructor method.

I was thinking making separate controllers for inbounds, outbounds, directs then instantiate the correct repository in the controller constructor. But that seems redundant in some ways.

I am wondering if I should have some sort of factory method that will load the proper repository when it's called from the controller. What do you folks think?

Note: I understand that implementing these design patterns are not required to be used for an application this simple but I want to learn it for educational purposes.

Generic repository interface:

namespace DirectOrderTracker.Services
{
    interface IRepository<T1> where T1 : class
    {
        IEnumerable<T1> GetAllSales(string commodity);
        void EditSale(T1 obj);
        void SaveChanges();
    }
}

Main controller:

namespace DirectOrderTracker.Controllers
{
    public partial class GridController : Controller
    {
        private SalesService salesService;
        private DirectRepository directs;
        private InboundRepository inbounds;
        private OutboundRepository outbounds;
        public GridController()
        {
            salesService = new SalesService(new OrdersEntities());

< start >Here is where I want to improve my code:

            directs = new DirectRepository(new OrdersEntities());
            inbounds = new InboundRepository(new OrdersEntities());
            outbounds = new OutboundRepository(new OrdersEntities());

< end >

        }
        /*
         * 
         *OUT-BOUND SALES
         * 
         */
        public ActionResult AllOutBounds([DataSourceRequest] DataSourceRequest request, DateTime? fromDate, DateTime? toDate, string commodity)
        {
            var result = outbounds.GetAllSales(commodity);

            return Json(result.ToDataSourceResult(request), JsonRequestBehavior.AllowGet);
        }
        [AcceptVerbs(HttpVerbs.Post)]
        public ActionResult EditOutBounds([DataSourceRequest] DataSourceRequest request, OutboundSalesViewModel sale)
        {
            if (sale != null && ModelState.IsValid)
            {
                outbounds.EditSale(sale);
                outbounds.SaveChanges();
            }
            return Json(new[] { sale }.ToDataSourceResult(request, ModelState));
        }
        /*
         * 
         *IN-BOUND SALES
         * 
         */
        public ActionResult AllInBounds([DataSourceRequest] DataSourceRequest request, DateTime? fromDate, DateTime? toDate, string commodity)
        {
            var result = inbounds.GetAllSales(commodity);

            return Json(result.ToDataSourceResult(request), JsonRequestBehavior.AllowGet);
        }
        [AcceptVerbs(HttpVerbs.Post)]
        public ActionResult EditInBounds([DataSourceRequest] DataSourceRequest request, InboundSalesViewModel sale)
        {
            if (sale != null && ModelState.IsValid)
            {
                inbounds.EditSale(sale);
                inbounds.SaveChanges();
            }
            return Json(new[] { sale }.ToDataSourceResult(request, ModelState));
        }
        /*
         * 
         *DIRECT SALES
         * 
         */
        public ActionResult AllDirects([DataSourceRequest] DataSourceRequest request, DateTime? fromDate, DateTime? toDate, string commodity)
        {
            var result = directs.GetAllSales(commodity);

            return Json(result.ToDataSourceResult(request), JsonRequestBehavior.AllowGet);
        }

        [AcceptVerbs(HttpVerbs.Post)]
        public ActionResult EditDirects([DataSourceRequest] DataSourceRequest request, DirectSalesViewModel sale)
        {
            if (sale != null && ModelState.IsValid)
            {
                directs.EditSale(sale);
                directs.SaveChanges();

                sale.Margin = Calculations.MarginCalc(sale.Qty, sale.UNITPRICE, sale.UNITCOST, sale.POFrghtRate);
            }
            return Json(new[] { sale }.ToDataSourceResult(request, ModelState));
        }

        public ActionResult GetSalesPeople()
        {
            return Json(salesService.GetAllSalesPeople(), JsonRequestBehavior.AllowGet);
        }

        public ActionResult GetAllCommodities()
        {
            return Json(salesService.GetAllCommodities(), JsonRequestBehavior.AllowGet);
        }
    }
}

Sample Inbound Repository:

public class InboundRepository : IRepository<InboundSalesViewModel>
    {
        private OrdersEntities entities;
        public InboundRepository(OrdersEntities entities)
        {
            this.entities = entities;
        }
        public IEnumerable<InboundSalesViewModel> GetAllSales(string commodity)
        {
            return entities.CustPOLines
                           .Where(x => x.WAREHOUSE != "~D")
                           .OrderByDescending(x => x.ModifyDate)
                            .Select(s => new InboundSalesViewModel
                            {
                                CustPOLineID = s.CustPOLineID,
                                VendDesc = s.Vendor.VendDesc,
                                FreshouseRefNum = s.FreshouseRefNum,
                                Qty = s.Qty.HasValue ? s.Qty.Value : default(decimal),
                                Cost = s.Cost.HasValue ? s.Cost.Value : default(decimal),
                                CostAlt = default(decimal),
                                PPROREFDATE = s.PPROREFDATE,
                                CarrierDesc = s.CarrierDesc,
                                LineFrghtRate = s.LineFrghtRate.HasValue ? s.Cost.Value : default(decimal),
                                VendPurchPONum = s.VendPurchPONum,
                                POLineComment = s.POLineComment,
                                Commodity = s.Product.Commodity,
                                LoadNumber = s.LoadNumber,
                                ProdDesc = s.Product.ProdDesc,
                                TYPE = s.TYPE,
                                Buyer = s.Buyer
                            })
                        .ToList();
        }

        public void EditSale(InboundSalesViewModel sale) {
            var existingSale = entities.CustPOLines.Find(sale.CustPOLineID);
            existingSale.POLineComment = sale.POLineComment;
            entities.Entry(existingSale).State = EntityState.Modified;
        }

        public void SaveChanges()
        {
            entities.SaveChanges();
        }

    }

Perhaps Dependency Injection is what I'm looking for? http://blog.agilistic.nl/a-step-by-step-guide-to-using-ninject-for-dependancy-injection-in-c-sharp/

\$\endgroup\$
  • 1
    \$\begingroup\$ Yes, I would suggest reading through a good DI tutorial since it seems like your structure would benefit from it. \$\endgroup\$ – Ron Beyer Nov 22 '16 at 16:11
  • \$\begingroup\$ @RonBeyer I went through and added Ninject to inject my repositories into the controller but now I have three separate repositories coming into the constructor. Maybe I'm just over thinking this whole thing. \$\endgroup\$ – Waragi Nov 22 '16 at 17:18
  • \$\begingroup\$ It is difficult to say you are doing it right or wrong, you could use property injection if you don't want to have them as parameters to your constructor. I don't know NInject very well, but if you search Google for NInject Property Injection it looks like it has some good references. \$\endgroup\$ – Ron Beyer Nov 22 '16 at 19:37
  • \$\begingroup\$ If you are at learning stage I would propose to check out Command Query Separation principle, it could help you to think in use cases instead of creating proxy classes and services whose are in your current example already violating some good practices (ISP, SOC, etc.). I don't want to SE cross post but please do read this and ... \$\endgroup\$ – kayess Nov 23 '16 at 15:53
  • \$\begingroup\$ ... this A's of mine where I have tried to extract basics of CQS. \$\endgroup\$ – kayess Nov 23 '16 at 15:57
1
\$\begingroup\$

Along with concrete type instanciation in the code you want to improve, there is another issue - Over-instantiation

There are four object always instantiated in constructor when most of them are not in use.

e.g. - In AllOutBounds action only outbounds is used - In AllDirects action only directs is used - etc

This can be resolved by extracting actions depends on different repos to separate controllers and injecting relevant dependenies into constactor.

e.g.

public interface IOutboundRepository : IRepository<OutboundSalesViewModel> {}

public class OutBoundsConroller : Controller
{
   private IOutboundRepository outbounds;

   public OutBoundsConroller(IOutboundRepository repo)
   {
     outbounds = repo;
   }

    public ActionResult AllOutBounds([DataSourceRequest] DataSourceRequest request, DateTime? fromDate, DateTime? toDate, string commodity)
    {
        var result = outbounds.GetAllSales(commodity);

        return Json(result.ToDataSourceResult(request), JsonRequestBehavior.AllowGet);
    }

    [AcceptVerbs(HttpVerbs.Post)]
    public ActionResult EditOutBounds([DataSourceRequest] DataSourceRequest request, OutboundSalesViewModel sale)
    {
        if (sale != null && ModelState.IsValid)
        {
            outbounds.EditSale(sale);
            outbounds.SaveChanges();
        }
        return Json(new[] { sale }.ToDataSourceResult(request, ModelState));
    }
}

If in some methods you need additional dependencies, you can inject them into that methonds (instead of injecting them into constructor)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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