Random Selection of two different objects causes objects to reference same value

Question

I'm trying to write a function that will randomly suggest an origin and a destination to travel between two cities or between the same city but different zip codes. I have a class to do this:

public class CityInfo
{
    public string CityName { get; set; }
    public string State { get; set; }
    public string Zip { get; set; }
    public IEnumerable<string> ZipCodes { get; set; }

    private static readonly IEnumerable<CityInfo> Cities = new []{
        new CityInfo{
            CityName = "New York",
            State = "NY",
            ZipCodes = new[]{"10001", "10002", "10003", "10004", "10005"}
        },
        new CityInfo
        {
            CityName = "Washington",
            State="DC",
            ZipCodes = new []{"20001", "20002", "20003", "20004","20005"}
        }
    };

    private static T GetRandom<T>(IEnumerable<T> list, Random generator)
    {
        return list.ToArray()[generator.Next(list.Count() - 1)];
    }

    public static CityInfo GetCity(Random random)
    {
        var city = GetRandom(Cities, random);
        city.Zip = GetRandom(city.ZipCodes, random);
        return city;
    }

}

The problem I am having is when I go to select two cities with the Get City method both the cities come out to the same value. I'm calling them as follows:

var random = new Random();
var originCity = CityInfo.GetCity(random);
var destinationCity = CityInfo.GetCity(random);

When i set a break point before calling the desination city get city function the value of origin city is different than when the function call completes so I am fairly convinced the originCity variable is referencing destinationCity somewhere I am just not sure why or how to fix it

you reinitlize your random number generator each time - this will give the same 'random' number — pm100, Nov 26 '14 at 17:27
that's not actually what is happening, the value is different in each city, but when destination city is set the value of origin city changes is the problem, but just to be sure i edited my code and it has exactly the same behavior — theDarse, Nov 26 '14 at 17:32
If any one else is having this issue the problem was actually caused by the city variable in the DetCity being passed as a reference from the Cities IEnumerable. If the same city was chosen then the reference was passed back and changing the zip in one item such as destination city will also change it in origin city because both object reference the same array element causing them to both be the same if the same city is chosen — theDarse, Nov 26 '14 at 18:00
As you updated your question not to be the same issue as the duplicate I have reopened your question. Please post your solution you found as an answer so other people can benefit from what you found. — Scott Chamberlain, Nov 26 '14 at 21:36
`Next` on the randomizer expects an exclusive max, not an inclusive one. So instead of `generator.Next(list.Count() - 1)`, it just should be `generator.Next(list.Count())`. For example, `generator.Next(3)` will give you 0, 1, or 2. — Joe Enos, Nov 26 '14 at 21:38
To add on to what Joe said, that is the entire reason it is a exclusive max, so you can do `.Count` or `.Length` and not need to subtract one to use it as a indexer in an array. — Scott Chamberlain, Nov 26 '14 at 21:40

score 1 · Accepted Answer · answered Nov 26 '14 at 23:41

The origin city is the same as the destination about half the time in your example, because there are only two CityInfo objects in the list of Cities. When the same city is chosen for both origin and destination the object is the same and therefore the Zip property selected by GetRandom(city.Zipcodes, random) is set again by the destination calling set on Zip a second time.

If you want to allow both origin and destination city to be the same CityName but with different Zip you will need to create a copy of the object (at least for the origin). Note you may still end up with origin equal to destination, randomly.

If you can require origin and destination to be different CityNames, then the following code will select random origin and destination CityInfo objects.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace stackOverflowRandomRefTest
{
    class Program
    {
        static void Main(string[] args)
        {
            var random = new Random();
            for (int i = 0; i < 10; i++)
            {
                var originCity = CityInfo.GetCity(random, null);
                var destinationCity = CityInfo.GetCity(random, originCity);
                Console.WriteLine(String.Format("orig: {0} {1}",originCity.CityName, originCity.Zip));
                Console.WriteLine(String.Format("dest: {0} {1}",destinationCity.CityName, destinationCity.Zip));
            }
            Console.ReadKey();
        }
    }
    public class CityInfo
    {
        public string CityName { get; set; }
        public string State { get; set; }
        public string Zip { get; set; }
        public IEnumerable<string> ZipCodes { get; set; }

        private static readonly IEnumerable<CityInfo> Cities = new[]{
        new CityInfo{
            CityName = "New York",
            State = "NY",
            ZipCodes = new[]{"10001", "10002", "10003", "10004", "10005"}
        },
        new CityInfo
        {
            CityName = "Washington",
            State="DC",
            ZipCodes = new []{"20001", "20002", "20003", "20004","20005"}
        }
        };

        private static T GetRandom<T>(IEnumerable<T> list, Random generator)
        {
            int n = generator.Next(list.Count());
            Console.WriteLine(n);
            return list.ToArray()[n];
        }

        public static CityInfo GetCity(Random random, CityInfo notThisCity)
        {
            var city = GetRandom(Cities, random);
            while (city == notThisCity)
            {  // if there is only one city this is infinite...
                city = GetRandom(Cities, random);
            }
            city.Zip = GetRandom(city.ZipCodes, random);
            return city;
        }

    }
}

Random Selection of two different objects causes objects to reference same value

1 Answers1