2

In my program, a product (or an ingredient) needs to update its own parents prices, and those parents have to do the same for their parents and so on.

I've wrote a method like this :

public static Price CalculatePrice(this IProduct pro)
{
    /// calculation stuff
}

private static SystemContext innerContext;

/// <summary>
/// In the main controller this method called for updates
/// </summary>
/// <param name="pro"></param>
/// <param name="sc"></param>
public static void UpdatePrice(this IProduct pro, ref SystemContext sc)
{
    if (sc.Relations.Where(t => t.SubProduct.ID == pro.ID).Any())
    {

        // If this returns any, this means there are some products using this product as their sub-product.
        var list = sc.Relations.Where(t => t.SubProduct.ID == pro.ID).ToList();

            ConcurrentQueue<Relation> quee = new ConcurrentQueue<Relation>(list);

            innerContext = new SystemContext();

            Task task = new Task(() => UpdatePrice(ref quee));
            task.Start();
    }
}


private static void UpdatePrice(ref ConcurrentQueue<Relation> queue)
{
    Relation val;

    while (queue.TryDequeue(out val))
    {
        val.Product.Price = val.Product.CalculatePrice();

        var list = innerContext.Relations.Where(t => t.SubProduct.ID == val.Product.ID);

        if (list.Any())
        {
            ConcurrentQueue<Relation> quee = new ConcurrentQueue<Relation>(list);
            Task task = new Task(() => UpdatePrice(ref quee));
            task.Start();

        }
    }
}

Although, first level of parent products get updated, second level are not.

Is there a better logic to do this ?

And by the way, any of the lowest level products have approx 1000 parents (recursively). Which means time consuming (because of the calculation of price). So it would be perfect if you consider the time also when you give advice...

Edit

While i was doing some test. I figured that, some lowest level prodducts have 4000 parents.

paroxit
  • 627
  • 2
  • 12
  • 30
  • Have you considered using recursion? Also, why would you need a thread for each parent? – Yuval Itzchakov Jan 04 '15 at 16:41
  • Yes. I got StackOverflow exception without the multithreading. – paroxit Jan 04 '15 at 16:42
  • 1
    Oh, now i see that you have 1000 parents. Are those parents only accessible be recursively traversing each object? See [this](http://stackoverflow.com/questions/141467/recursive-list-flattening) for an example. – Yuval Itzchakov Jan 04 '15 at 16:43
  • Don't use `new Thread` unless your threads are going to last a long time, and there are limited number of them. Else use the `Threadpool`. However you should just use `TPL`. – Aron Jan 04 '15 at 17:33
  • Within the all of these methods, Entity Framework takes action. So it has to be divided into different threads. – paroxit Jan 04 '15 at 17:54
  • I don't understand the code example. It seems buggy. You have a `static` named `innerContext` which you initialize prior to starting a `Task`. But how many times do you call the top-level `UpdatePrice(IProduct, SystemContext)` method? If more than once, how do you avoid conflicts with the context assignment? Also, the `ref` passing seems superfluous; why do you use `ref`? Finally, why use `ConcurrentQueue` instead of plain `Queue`? If you really have concurrent access, then it's possible for some thread to just give up before it's done; if you don't, then you don't need `ConcurrentQueue`. – Peter Duniho Jan 04 '15 at 19:54
  • UpdatePrice(IProduct, ref SystemContext) is called once, from another class. And a SystemContext passed into it with ref keyword in order to avoid any unnecessary memory consumption . And then it calls UpdatePrice(ref ConcurrentQueue) with Task. By the way ConcurrentQueue is used because i was using MTA before :) – paroxit Jan 04 '15 at 21:30

1 Answers1

2

I think the solution might be to process the data iteratively rather than recursively. You can do that by adding all products you are interested to a list and keep adding to that list as you are processing the products in it.

Something like this:

public static void UpdatePrice(this IProduct pro, ref SystemContext sc)
{
    var relations = sc.Relations.Where(t => t.SubProduct.ID == pro.ID).ToList();

    for (int i = 0; i < relations.Count; i++)
    {
        relations[i].Product.Price = relations[i].Product.CalculatePrice();
        relations.AddRange(sc.Relations.Where(t => t.SubProduct.ID == relations[i].Product.ID)); 
    }    
}
NeddySpaghetti
  • 13,187
  • 5
  • 32
  • 61
  • But within your suggestion, there is no part that updates the price of the second or any above level parent. – paroxit Jan 04 '15 at 17:19
  • they'll be processed in subsequent iterations of the loop, just add them to the list in the body of the loop – NeddySpaghetti Jan 04 '15 at 17:30
  • How can i add parents to that list without recursion? That doesn't sound possible. – paroxit Jan 04 '15 at 17:45
  • can you post some sample data, for say 5 products and I can try and code the example. – NeddySpaghetti Jan 04 '15 at 17:57
  • Have a look at the link in @Yuval Itzchakov's comment, it looks like a better solution – NeddySpaghetti Jan 04 '15 at 18:07
  • @paroxit: based on your question's code, I would say in Ned's example you just replace "calculate price" comment with `relations[i].Product.Price = relations[i].Product.CalculatePrice();` and "add more relations to the list" with `relations.AddRange(sc.Relations.Where(t => t.SubProduct.ID == relations[i].Product.ID));`. (His misspelling of "relations" in the initial variable declaration notwithstanding :) ). See also my comment to the question regarding threading bugs. – Peter Duniho Jan 04 '15 at 19:50
  • They are real relations, realations :) – NeddySpaghetti Jan 04 '15 at 21:16
  • Nice one @NedStoyanov :) And by the way, Peter, adding range seems to be working for some, but for some other entities that have over 4000 parents, this is not possible... But i'm trying new methods by extending your and Ned's logic. I'll inform everyone if it'd work. – paroxit Jan 04 '15 at 21:38
  • This is actually working ! I was mistaken before because of the one entity that binded to itself as a subproduct in the db, which was putting this loop in endless situation and freezing the program. Even though i wasn't getting any stackoverflow ex. i thought that was it. So this is the solution, thanks guys ! – paroxit Jan 04 '15 at 22:09