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 written this ActionResult to model an Order that goes through. I couldn't think of another way to write this without using context Db<Sets>. It runs kinda slow, taking around 4 seconds to complete. Is there way to optimize the code or is this kinda has to happen since I'm doing so many inserts?

public ActionResult New(CustomerOrderModel model)
{
    ViewBag.LimitNotExceeded = checkLimit();
    if (ModelState.IsValid)
    {
        int prepaid;
        int userIdLoggedIn = WebSecurity.CurrentUserId;
        try
        {
            //
            //Add Customer
            using (var ctx = new CustomerContext())
            {
                ctx.CustomerInfos.Add(model.CustomerModel);
                ctx.SaveChanges();
            }

            //
            //Add Order
            model.OrderInfo.UserProfileUserID = userIdLoggedIn;
            model.OrderInfo.Customer_id = model.CustomerModel.id;
            model.OrderInfo.DateCreated = DateTime.Now.ToLocalTime();
            using (var ctx = new OrdersContext())
            {
                ctx.OrderInfos.Add(model.OrderInfo);
                ctx.SaveChanges();
            }

            //
            //Add Code
            key = "s121sd";
            KeyCode TheKey = new KeyCode();
            TheKey.code = key;
            TheKey.Customer_id = model.CustomerModel.id;
            TheKey.nif = model.CustomerModel.nif;
            TheKey.UserProfileUserId = userIdLoggedIn;
            TheKey.DateCreated = DateTime.Now.ToLocalTime();
            using (var ctx = new CodesContext())
            {
                ctx.CodesInfos.Add(TheKey);
                ctx.SaveChanges();
            }

            //
            //Assign and Create debts
            using (var ctx = new UsersContext())
            using (var dbx = new DebtsContext())
            {
                prepaid = ctx.UserProfiles.Find(userIdLoggedIn).Prepaid;
                if (prepaid > 0)
                {
                    --prepaid;
                    string query = "UPDATE dbo.UserProfile SET Prepaid='" + prepaid + "' WHERE UserId='" + userIdLoggedIn + "';";
                    ctx.Database.ExecuteSqlCommand(query);
                }
                else
                {
                    Debts NewDebt = new Debts();
                    NewDebt.paid = false;
                    NewDebt.DateCreated = DateTime.Now.ToLocalTime();
                    NewDebt.UserProfileUserId = userIdLoggedIn;
                    NewDebt.Order_id = model.OrderInfo.id;
                    dbx.DebtInfos.Add(NewDebt);
                    int debts = dbx.DebtInfos.Count(q => q.UserProfileUserId == userIdLoggedIn);
                    if (debts <= ctx.UserProfiles.Find(userIdLoggedIn).MaxDebt)
                    {
                        //
                        //Warn them when they reach their last
                    }
                    dbx.SaveChanges();
                    string query = "UPDATE dbo.UserProfile SET Prepaid='" + prepaid + "' WHERE UserId='" + userIdLoggedIn + "';";
                    ctx.Database.ExecuteSqlCommand(query);
                }
            }
            //DateTime a = DateTime.FromBinary(model.OrderInfo.Timestamp);
            return RedirectToAction("Index", "Home");
        }

        catch (DataException ex)
        {
            ViewBag.ChangeResult = ex;
            ModelState.AddModelError("", "Unable to save changes. Try again, and if the problem persists see your system administrator.");
        }
    }
    return View(model);
}
share|improve this question
    
I think you could go with wrapping the context in a "Unit Of Work" and the UnitOfWork in a "Repository" patterns. asp.net/mvc/tutorials/getting-started-with-ef-5-using-mvc-4/… –  Alex Peta Jan 7 '14 at 14:44
    
Just want to mention that DbContext is an implementation of the repository and unitofwork patterns in itself. –  Helge Heldre Jan 8 '14 at 7:52

1 Answer 1

I'm not sure i understand exactly why you have multiple contexts. If you keep to one context you can do one SaveChanges(). That should save you some time in storing data.

Do all your SaveChanges() take equal amount of time?

You could add a timer inside the contexts to print out a console message with time to confirm how long a using statement takes to come to an end.

If you are relying on how long it takes to show in the view, i suggest using a tool like Glimpse or MiniProfiler to give you feedback on time spent rendering/servertime.

share|improve this answer
    
My understanding of having multiple context is so I can access/store models from/into the DB. So some SaveChanges() take longer than others. I was sceptical of using so many context in one ActionResult so I posted here. Is this even the correct way to do inserts? –  GivenPie Jan 7 '14 at 19:49
    
if you let your view sit around and wait for the respons for this I dont think its a good idea. With so many separate contexts you could let them work in threads as long as they dont interfere with eachother. –  Helge Heldre Jan 8 '14 at 7:50
    
I think that the usage of SQL queries in the controller isent the best option. If you make your own repository/unitofwork you use it to handle such special cases. In my mind this break with single responsibility principle (SRP) and separation of concerns. –  Helge Heldre Jan 8 '14 at 7:57

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.