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.

We have a table with periods, and other child tables with these periods and values for combinations of product + location.

Based on selected periods, I would like to add any missing rows in the child tables. I have come up with the following code, which seems to work just fine. But I would like to know if there is a simpler or more efficient way of doing this, since I have to repeat this for a number of tables.

public static void SeedActivityData(int startPeriod, double endPeriod, IList<DAL.OMS_StockStatus> stockStatus)
{
    DAL.EFDbContext dbFilter = new DAL.EFDbContext(0, stockStatus.FirstOrDefault().Product, startPeriod, endPeriod);
    var period = dbFilter.Periods.Select(b => b.PeriodID);
    foreach (var item in stockStatus)
    {
        IEnumerable<int> missingFAF = period.Except(dbFilter.OMS_Forecast_Adjustment.Where(a => a.SiteID == item.SiteID).Select(b => b.PeriodID));

        var modelFAF = db.OMS_Forecast_Adjustment;
        foreach (int periodID in missingFAF)
        {
            var modelItem = new OMS_Forecast_Adjustment { PeriodID = periodID, Product = item.Product, SiteID = item.SiteID, Value = 0 };
            modelFAF.Add(modelItem);
        }

        IEnumerable<int> missingRAF = period.Except(dbFilter.OMS_Receipts_Adjustment.Where(a => a.SiteID == item.SiteID).Select(b => b.PeriodID));

        var modelRAF = db.OMS_Receipts_Adjustment;
        foreach (int periodID in missingRAF)
        {
            var modelItem = new OMS_Receipts_Adjustment { PeriodID = periodID, Product = item.Product, SiteID = item.SiteID, Value = 0 };
            modelRAF.Add(modelItem);
        }

        db.SaveChanges();
    }
}
share|improve this question
1  
Could you add some more detailed information? It would be helpful to see some examples of the situations you are trying to address. For instance showing the data/collection before, then after. Thanks! My first thought though is that it looks like you repeat the same iteration and adding twice. You can probably pull that out into a separate method. –  Steve Michael Oct 3 '14 at 9:58

1 Answer 1

up vote 2 down vote accepted

Looks to me like you should create an Add method to both the OMS_Forecast_Adjustment and OMS_Receipts_Adjustment classes.

you are really adding things to the db all wrong, you are creating new objects in your code when you don't have to, I think that you really want to add items to a collection variable inside your db object.

it should be an InsertRecord method

this:

    var modelFAF = db.OMS_Forecast_Adjustment;
    foreach (int periodID in missingFAF)
    {
        var modelItem = new OMS_Forecast_Adjustment { PeriodID = periodID, Product = item.Product, SiteID = item.SiteID, Value = 0 };
        modelFAF.Add(modelItem);
    }

Would turn into this

foreach (int periodID in missingFAF)
{
    db.Insert_OMS_Forcase_Adjustment(periodID, item.prodeuct, item.SiteID, 0);
}

so you actually do your insertion inside of a method in the class that you already have.

This removes the creation of 2 extraneous local variables and puts responsibility where it belongs inside of a method in the object that needs to perform the action.

This can be done on both Foreach Loops

share|improve this answer
    
Thanks Malachi. But how can I implement the InsertRecord method? Or should I now ask this in StackOverflow forum? –  Randeep Singh Oct 4 '14 at 18:03
    
it's a different structure than what you have here, but it is doing pretty much the same thing. I don't know how they would react to that question on StackOverflow. and as far as a StackOverflow forum, I didn't know there was one.... –  Malachi Oct 4 '14 at 20:30

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.