-1

So i have a program which starts to build a dictionary of possible paths from a given starting point back to the same point using an exact number of steps.

I start out by adding a dictionary row for each unique combination of start and end steps, and then i have a method which basically finds all unique combinations that can fill in the remaining steps in between.

It looks something like this:

        private int CreatePathsWithAllSteps(int pathKey)
    {
        try
        {
            //Make sure we reiterate through the list of paths for all the configured steps to complete the paths.
            for (int i = 0; i < MaxSteps - 2; i++)
            {
                //Create a fresh list of new paths with each complete iteration so we're not duplicating added paths
                Dictionary<int, TravelPath> newPaths = new Dictionary<int, TravelPath>();

                foreach (KeyValuePair<int, TravelPath> path in Paths)
                {
                    //We need to find the list of Destination Pairs we can use with the existing path
                    Dictionary<string, DestinationPair> pairsToUse = FindDestinationPairsToUseNext(path, i+1);
                    bool isfirstiteration = true;

                    //***Issue*** 1 - Creating the copy of the KVP so I can reuse it to create all possible combinations of it with the next step in the path.
                    //I use the values of the KVP from the dictionary i'm iterating through. This seems to create them as reference paths back to the original dictionary.  
                    KeyValuePair<int, TravelPath> pathCopy = new KeyValuePair<int, TravelPath>(path.Key, path.Value);
                    
                    foreach (KeyValuePair<string, DestinationPair> TP in pairsToUse)
                    {
                        //Make sure we're using already existing paths first and then create new ones for all other options. 

                        if (isfirstiteration)
                        {
                            //First time through use path already created at this point
                            if (path.Value.Path[i].Name != TP.Value.Name)
                            {
                                //Destination Pair is different to the previous pair so add it
                                path.Value.Path[i + 1] = TP.Value;
                                isfirstiteration = false;
                            }
                            
                        }
                        else
                        {
                            //for this one, create a new copy of the path we're working on and then update with next step
                            if (pathCopy.Value.Path[i].Name != TP.Value.Name)
                            {
                                //***Issue*** 2 - When I use the value of the copied KVP to create a new KVP to add to the dictionary this also seems to create a reference back to the copy and by extension the original value from the original dictionary we're iterating through. 
                                KeyValuePair<int, TravelPath> tempTP = new KeyValuePair<int, TravelPath>(pathKey, pathCopy.Value);
                                //***Issue*** 3 - I then edit the value of the KVP created in the previous line, and this changes the value in the original main Paths dictionary.
                                tempTP.Value.Path[i + 1] = TP.Value;
                                if (!PathValueComparer(tempTP, newPaths, Paths))
                                {
                                    newPaths.Add(pathKey, pathCopy.Value);
                                    pathKey++;
                                }
                                
                            }
                            
                        }
                    }

                   
                }

                foreach(KeyValuePair<int, TravelPath> path in newPaths)
                {
                    Paths.Add(path.Key, path.Value);
                }
            }
            return pathKey;
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
            return pathKey;
        }

    }

I've added some extra comments with the ***Issue*** prefix so you can see where i think the issue is and what i believe has happened here.

Now it's been 3 or 4 years since i last did regular coding so I'm a bit rusty (please don't judge my code too much lol), but I did remember having similar issues when dealing with dictionaries before, and I seem to remember the solution was to have a custom dictionary that implemented both IDictionary and ICloneable, so you'd call .Clone() to create an unreferenced copy and could then do temporary edits to the dictionary and use the values without them being referred back to your source dictionary.

I thought I would try doing something similar with KVP but it doesn't seem that there is an interface for KVP you can actually use as the basis for a custom implementation.

I feel like I could probably just create a cloneable dictionary, then clone it before taking the copy of the KVP I need from the cloned version, but for this piece of work I don't really have a need for an entire dictionary to be cloneable, just the KVP, so it seems a little wasteful

So i guess the question is, is there a better way of breaking the reference between the values of a KVP and a Dictionary?

mattisrowe
  • 149
  • 2
  • 14
  • 3
    This is nothing to do with [`KeyValuePair`](https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.keyvaluepair-2?view=net-5.0) (which is a *struct* and therefore always copies by value) and everything to do with your `TravelPath` *class*, which is copied by reference. What exactly is your question? – Charlieface Aug 29 '21 at 00:55
  • @charlie ahh ok, in that case i guess my question would be how i ensure that when i do `tempTP.Value.Path[i + 1] = TP.Value;` i ensure that the dictionary value `tempTP` was copied from is not edited, which it currently is at the moment..... i guess the solution to my problem then is to implement ICloneable in my TravelPath Class and then i can clone the value of the KVP..... sounds like it would solve the issue. – mattisrowe Aug 29 '21 at 01:11
  • 1
    Indeed, but we have no idea anything about `TravelPath` so cannot advise – Charlieface Aug 29 '21 at 01:33
  • 1
    `new KeyValuePair(path.Key, path.Value);` -> `new KeyValuePair(path.Key, new TravelPath(...));` ? – Caius Jard Aug 29 '21 at 08:01
  • @caiusJard Ooooh that worked nicely thanks. – mattisrowe Aug 29 '21 at 20:21

1 Answers1

0

The essential problem is that though you have two different KeyValuePair in memory, they refer to the same TravelPath class

 //this line
KeyValuePair<int, TravelPath> pathCopy = new KeyValuePair<int, TravelPath>(path.Key, path.Value);

//sets up this in memory

123 <-- path.Key, path.Value --> TravelPathX <-- pathCopy.Value, pathCopy.Key --> 123

The value types are cloned so there are two distinct 123, but both Value properties refer to the same instance of TravelPath. Modifying properties of the TravelPath instances means both path and pathcopy see it

Without seeing TravelPath it's hard to know further but the simple solution is to just make a new TravelPath and copy over the properties you want. Remember to clone anything you want to modify, eg if TravelPath has a list, it needs newing too (and if you're going to modify theobjects inside the list instead of deleting and making new ones, then those objects need newing too..)

//this line
var pathCopy = new KeyValuePair<int, TravelPath>(path.Key, new TravelPath { Name = path.Value.Name ... });

//Would set up this in memory
123 <-- path.Key, path.Value --> TravelPathX
                                 TravelPathY <-- pathCopy.Value, pathCopy.Key --> 123
Caius Jard
  • 72,509
  • 5
  • 49
  • 80