C# Infinite While Loop using ContainsValue with nested Dictionary

Question

I have this script in PHP where I use

while( in_array(array('x' => $x, 'y' => $y), $worldMap) ){ ... }

to check if my worldMap already have a room at those XY positions. IF TRUE I randomize either X or Y and the WHILE loop check again with the new values and so on, IF FALSE I populate the worldMap array with last XY generated.

Now, I'm trying to rewrite that code in C# but I'm getting an infinite Loop.

Here's my current Code:

public int nbRooms = 10;

private Dictionary<int, Dictionary<string, int>> worldMap = new Dictionary<int, Dictionary<string, int>>();
private Dictionary<string, int> roomXY = new Dictionary<string, int>();

private string[] arrayXY = {"X","Y"};
private int[] arrayNbr = {-1,1};
private int X = 0;
private int Y = 0;

    for(int i = 0; i <= nbRooms; i++)
    {
        while(worldMap.ContainsValue(roomXY))
        {
            string XorY = arrayXY[Random.Range(0, 2)];

            switch(XorY)
            {
            case "X": X += arrayNbr[Random.Range(0, 2)];
                break;
            case "Y": Y += arrayNbr[Random.Range(0, 2)];
                break;
            }

            roomXY.Clear();
            roomXY.Add("X", X);
            roomXY.Add("Y", Y);
        }

        worldMap.Add(i, roomXY);
    }

Answer 1

The basic issue here is that, by default, a comparison between two reference type objects simply compares the reference itself. While you change the contents of the roomXY object, you do not change the reference itself (i.e. the actual object remains the same), and so once you have added the object to your worldMap dictionary once, it is always there when you check the next time through the loop.

A very good illustration of why when porting code it's important to port the intent but not necessarily the exact implementation, due to differences in the way the language handles things.

In fact, based on the code you've posted, it seems as though you probably don't want to use a dictionary class anywhere in this case. It can be made to work using dictionary objects, but you aren't really taking advantage of the dictionary-like nature of those data structures. It seems like you are using dictionaries here more because semantically they seem to operate similarly to the data structures you were using in PHP, but in fact C# offers other language features that would probably be more appropriate.

For example, you could have written your code like this:

struct Room
{
    public readonly int X;
    public readonly int Y;

    public Room(int x, int y) { X = x; Y = y; }
}

public int nbRooms = 10;

private Room[] worldMap = new Room[nbRooms];

private string[] arrayXY = {"X","Y"};
private int[] arrayNbr = {-1,1};
private int X = 0;
private int Y = 0;
private Room roomXY = new Room(X, Y);

    for(int i = 0; i <= nbRooms; i++)
    {
        while(Array.IndexOf(worldMap, roomXY) >= 0)
        {
            string XorY = arrayXY[Random.Range(0, 2)];

            switch(XorY)
            {
            case "X": X += arrayNbr[Random.Range(0, 2)];
                break;
            case "Y": Y += arrayNbr[Random.Range(0, 2)];
                break;
            }

            roomXY = new Room(X, Y);    
        }

        worldMap[i] = roomXY;
    }

Because of the way C# implements equality comparisons by default for value types (i.e. a struct), this will compare the actual contents of the roomXY value against the values found in worldMap.

Note: both your original implementation and the one above use a linear search in the worldMap data structure. For the small number of rooms here (10), this should be fine. But you should be aware that this can be very inefficient for larger sets of data. You will likely want to use a different approach to generate this data in that case (e.g. hash set, flags in a larger map data structure, shuffling, etc.).