C# - How to loop through a dictionary where Key is Int and the Value is a List, and based on condition, either add to list or create new entry

Question

This method takes in a list of massings, and attempts to group them according to their X and Y attributes. I attempted to do this by creating a dictionary Dictionary<int, List<Massing>> called buildingList where I add Massing objects to List<Massing> if the current Massingobject in the iteration has the same X and Y attributes as a Massingobject already in buildingList, If they are not, I create a new dictionary entry with the accumulated counter.

The error i am running into is Collection was modified, enumeration may not execute. From my shallow understanding, I think this problem is occurring because I am trying to alter the value list by adding more items to the value list during run time.

I am not sure if this is the only issue or if the bug runs deeper but any guidance would be greatly appreciated. Also if there is an entirely better (AND PLEASE SIMPLER) way to do this, like with Linq, and some cool GroupBy operation, or any other way that would be interesting to see.

public Dictionary<int, List<Massing>> GroupMassing(List<Massing> massings)
    {
        // create dictionary {key = id : value = List<Breps>}
        Dictionary<int, List<Massing>> buildingList = new Dictionary<int, List<Massing>>();

        // init counter to be used as key
        int counter = 1;

        for (int i = 0; i < massings.Count; i++)
        {
            // Get center points and x and y
            Massing myMassing = massings[i];
            Point3d cPnt = myMassing.CPnt;
            double xC = cPnt.X;
            double yC = cPnt.Y;

            // check if dic is empty
            if (buildingList.Count == 0)
            {
                List<Massing> myMassings = new List<Massing>() { myMassing };
                buildingList[counter] = myMassings;
            }

            // check the center point of current massing to all the center points of all the breps in the dictionary
            else
            {
                foreach (KeyValuePair<int, List<Massing>> entry in buildingList)
                {
                    List<Massing> list = entry.Value.ToList();
                    for (int j = 0; j < list.Count; j++)
                    {
                        Massing massing = list[j];
                        Point3d cPnt2 = massing.CPnt;
                        double xC2 = cPnt2.X;
                        double yC2 = cPnt2.Y;

                        if (xC2 == xC && yC2 == yC)
                        {
                            List<Massing> massingCopy = buildingList[counter];
                            massingCopy.Add(massing);
                        }
                        else
                        {
                            counter++;
                            List<Massing> myMassings2 = new List<Massing>() { myMassing };
                            buildingList[counter] = myMassings2;
                        }
                    }
                }

            }

        }

        return buildingList;
    }

If you add source for related code, it will be quite easy for us to put up a couple of tests for your code and get the issue solved. BR — Roar S., Jun 27 '21 at 20:14
First things first.. what's a Massing? If you're trying to group by a two-number point, why is your key a single number? — Caius Jard, Jun 27 '21 at 20:18
[link] (https://github.com/karimdaw1991/FacadeManager/tree/master/FacadeManager) here is the link to the repo, the code would be compiled into a .gha file for rhino3d's grasshopper software. — Karim Daw, Jun 27 '21 at 20:37
hi, @CaiusJard. A Massing is an object i made up that has an attribute called "CenterPoint". This would have an X and Y component of course. I guess the key would be equivalent to the ID of the massing list. So as an example `{ Key=1 , Value = [ massing1, massing2, massing3] } - all their x and y attributes are the same { Key=2 , Value = [ massing4, massing5, massing6] } - all their x and y attributes are the same` — Karim Daw, Jun 27 '21 at 20:44

Caius Jard · Accepted Answer · 2021-06-27T21:02:00.943

like with Linq, and some cool GroupBy operation

I think the code you have can essentially be replaced with:

massings.GroupBy(m => new { m.CPnt.X, Ym.CPnt.Y }).ToDictionary(g => g.Key, g => g.ToList());

But it doesn't result in an Dictionary<int, List<Massing>> - I can't work out how grouping on a point having X and Y should logically result in a single int; it should remain an X and Y or, perhaps if you want a single int out of it, and know that e.g. Y will never exceed 10000, you could:

massings.GroupBy(m => m.CPnt.X * 10000 + Ym.CPnt.Y).ToDictionary(g => g.Key, g => g.ToList());

Which will take an X of 23 and a Y of 34 and produce 230034 (which is, if you think about it, just a basic way of coding two ints as one in such a way that you can get the original X/Y back with a divide/modulo op pair)

It might not be quite as performant as doing it directly yourself, which looks more like:

var d = new Massing[0].ToDictionary(x => new { m.CPnt.X, Ym.CPnt.Y }, x => new List<Massing>());

foreach(var m in massings){
   var k = new { m.CPnt.X, Ym.CPnt.Y };
   if(!d.ContainsKey(k))
     d[k] = new List<Massing>();
   d[k].Add(m);
}

We need a bit of a hack to make the dictionary; I don't believe it's possible to "get the type" of an anonymous type (because ATs are types written by the compiler) in order to directly declare a Dictionary<AT,List<T>> but we can have the compiler write the bits we need by turning a zero length array into an empty dictionary with a bit of LINQ, then fill it without LINQ. If you don't want to do this hackery with anonymous types you can take a look at making some kind of Tuple, or a record if your C# is modern enough (or a plain class if you want to override equals and hashcode yourself)

The reason i want to have a key to be a single number is because the Key would be the ID for the values containing the list of all massings in that entry that have the same x and y positions in space, a single key "1" could have 10 massings, all with the same x and ys, — Karim Daw, Jun 27 '21 at 21:10
But the key is then completely unrelated to the data you're grouping by, so you need another way of quickly finding what the single number is as you group. For example, let's say that a point of 23,47 has (via your method) come to have a key of 2 (because it was the second one you encountered). And you encounter *another* massing also at 23,47 while you're examining the 538th entry in the list. How do you go from what you have (23,47) to what you need (2) without looking through every single one? (The answer is you need another lookup, or you need a way of calculating the int) — Caius Jard, Jun 28 '21 at 05:35
(But I genuinely cant see the point of doing so, if you're just going to arbitrarily assign an int you might as well group, and then iterate the result, assigning the int after the group is done, not during. If the int truly is arbitrary it's also relatively pointless; you might as well just group by the key and not return a dictionary at all, just return a List of List of Massing, the index of the first list being the int, or even just return the grouping because it's conceptually already a list of lists) — Caius Jard, Jun 28 '21 at 05:42
Yes you are right, i see the redundancy. I managed to solve it by making a list of lists. Thank you for pointing that out to me and helping me understand that! — Karim Daw, Jun 28 '21 at 20:39

C# - How to loop through a dictionary where Key is Int and the Value is a List, and based on condition, either add to list or create new entry

1 Answers1