Search code examples
c#objectconstructorinstances

Object instance is overwriting the other instance but I cant see why


I was wondering if you could look at my code and see where I have gone wrong. Basically, I have created an object of type "UserFile"(my own object type) and am creating 2 instances of it and within the constructor of that object, I am calling a static class method. All goes well for them except the second instance overwrites the first one after the object constructor is called. I have stepped through the program and am thoroughly confused. I get the feeling im missing something very obvious here.

Here is the button on the form that creates the instances

    private void btnCompare_Click(object sender, EventArgs e)
    {
        if (lstFiles.CheckedItems.Count == 2)
        {
            file1 = new UserFile(((FileLocation)lstFiles.CheckedItems[0]).filePath);

            file2 = new UserFile(((FileLocation)lstFiles.CheckedItems[1]).filePath);
        }
    }

Here is the UserFile class with constructor

public class UserFile
{
    public Dictionary<int,Individual> fileIndividuals;
    public Dictionary<int, Family> fileFamilies;
    public Header fileHead;

    public UserFile(string _dir)
    {
        fileIndividuals = new Dictionary<int, Individual>();
        fileFamilies = new Dictionary<int, Family>();
        fileHead = new Header();
        ReadFromFile.Read(_dir);
        fileIndividuals = ReadFromFile.individuals;
        fileFamilies = ReadFromFile.families;
        fileHead = ReadFromFile.head;
    }
}

Here is the ReadFromFile method called by the UserFile class

static class ReadFromFile
{
    public static string filename = "";

    public static Header head;
    public static Individual individual;
    public static Dictionary<int, Individual> individuals = new Dictionary<int, Individual>();

    public static Family family;
    public static Dictionary<int, Family> families = new Dictionary<int, Family>();

    public static GedcomRecordEnum currentRecord = GedcomRecordEnum.None;
    public static GedcomSubRecordEnum currentFirstLvlRecord = GedcomSubRecordEnum.None;
    public static GedcomSecondLevelEnum currentSecondLvlRecord = GedcomSecondLevelEnum.None;

    static public void Read(string fileName)
    {
        individuals.Clear();
        families.Clear();
        head = null;
        if (File.Exists(fileName))
        {
            filename = fileName;
            StreamReader reader = new StreamReader(fileName);

            while (!reader.EndOfStream)
            {
                string currentLine = reader.ReadLine();
                Match m = Regex.Match(currentLine, "(?<index>[0-9]) (?<keyword>[A-Z_@0-9]+)(?: *)(?<detail>.*)");

                string debug = m.Groups["index"].ToString();

                switch (m.Groups["index"].ToString())
                {
                    case "0":
                        ProcessRootLevel(m.Groups["keyword"].ToString());
                        break;
                    case "1":
                        ProcessLevel1(m.Groups["keyword"].ToString(), m.Groups["detail"].ToString());
                        break;
                    case "2":
                        ProcessLevel2(m.Groups["keyword"].ToString(), m.Groups["detail"].ToString());
                        break;
                    case "3":
                        ProcessLevel3(m.Groups["keyword"].ToString(), m.Groups["detail"].ToString());
                        break;
                }
            }
            reader.Close();
        }
    }
}

Solution

  • The problem is the following static properties on the ReadFromFile class, if I assume by "overwritten" you mean that both instances of UserFile point to the same data:

     public static Dictionary<int, Family> families = new Dictionary<int, Family>();
     public static Dictionary<int, Individual> individuals = new Dictionary<int, Individual>();
     public static Header head;
    

    The problem lies in the constructor of UserFile on the use of static properties.

        ReadFromFile.Read(_dir);
        fileIndividuals = ReadFromFile.individuals; // <-- Uh-oh!
        fileFamilies = ReadFromFile.families;       // <-- Uh-oh!
        fileHead = ReadFromFile.head;               // <-- Uh-oh!
    

    What happens here is the member variables fileIndividuals, fileFamilies and fileHead are set to a reference of the individuals, families and head properties on the static ReadFromFile class, not a copy (as they are classes and not value types). So the next time ReadFromFile.Read() is called the static properties on ReadFromFile are updated (overwritten), but the prior instance of UserFile just points to the same static properties, ergo file1 and file2 will have the same data.

    So how would you fix this? Two options:

    1. Make ReadFromFile and instance class, and not a static one. Construct a new instance in the UserFile constructor and don't use any static properties.
    2. Make a copy of the data in individuals, families and head in the constructor of UserFile. "foreach" through each item, and copy it into a new dictionary.

    Simple explanation:

    When you do an assign (the = character in C#) if the object is a class, then the target is assigned a "pointer" (reference) to the right hand side. If it's a value type it's copied. Dictionary is a class, so you get a pointer and not a copy.

    Illustration in code:

    public static class MyStaticClass
    {
         public static List<string> MyList = new List<string> 
    }
    

    Elsewhere...

    public void MyMethod()
    {
        List<string> myList1 = MyStaticClass.MyList; 
        List<string> myList2 = MyStaticClass.MyList; 
    
        myList1.Add("Hello");  // Add to first list
        myList2.Add("World");  // Add to second list 
    
        foreach(string item in myList1) // print all items in the second list
        {
             Console.WriteLine("List 1: " + item); 
        }
    
        foreach(string item in myList2) // print all items in the second list
        {
             Console.WriteLine("List 2: " + item); 
        }
    } 
    

    The output of this will be:

    List 1: Hello
    List 1: World
    List 2: Hello
    List 2: World 
    

    But why? We only added 'World' to myList2. Well myList1 and myList2 point to the same thing. When we did myList1 = MyStaticClass.MyList we didn't get a copy of the item, just a reference to it.