0

I am reading in a list of pets from a file, I have a simple text file and it has each attribute o a new line. It reads through my first pet perfect and uses my dictionary to create the pet. However when it clears the two lists it clears those details for the first created pet and as they get read in for the second pet they end up having matching traits and this goes on for all 8 of my pets. I know it's when I clear my lists it clears it for my previously created pet, however why does it not write over the name as well? I'm very new to OOP so please forgive me if I sound completely incompetent.

public static List<Pet> LoadPetDetails(string filename)
    {
        RegisterPet("Sheep", typeof(Sheep));
        RegisterPet("Dog", typeof(Dog));
        RegisterPet("Cat", typeof(Cat));
        RegisterPet("Rabbit", typeof(Rabbit));

        StreamReader reader = new StreamReader(filename);
        string kind, name, id, breed, desc;
        int age;
        bool gender;
        petSize size;
        List<string> petTraits = new List<string>();
        List<string> require = new List<string>();
        List<Pet> loadedPets = new List<Pet>();

        try
        {
            //read in the count of how many pets in care currently
            int count = Convert.ToInt32(reader.ReadLine());

            //read in details for pet 1 through to we reach the count 
            for (int j = 1; j <= count; j++)
            {
                //clear both of these list from the previous pet
                petTraits.Clear();
                require.Clear();

                //read in the kind
                kind = reader.ReadLine();
                //read in name
                name = reader.ReadLine();
                //read in id
                id = reader.ReadLine();
                //read in the pet traits
                for (int i = 0; i <= 2; i++)
                    petTraits.Add(reader.ReadLine());
                //read in pet breed
                breed = reader.ReadLine();
                //read in the pet's description
                desc = reader.ReadLine();
                //read in list of requirments
                for (int i = 0; i <= 1; i++)
                    require.Add(reader.ReadLine());
                //read in age
                age = Convert.ToInt32(reader.ReadLine());
                //read in size
                size = ConvertToPetSize(reader.ReadLine());
                //read in gender
                gender = Convert.ToBoolean(reader.ReadLine());

                //create the pet and add it to the list of pets to return
                loadedPets.Add(Pet.CreatePet(kind, name, id, petTraits, breed, desc, require, age, size, gender));
            }
        }
        finally
        {
            reader.Close();
        }
        return loadedPets;
    }
S.M
  • 19
  • 1
  • 5

1 Answers1

0

Somewhere in CreatePet you probably have some code like pet.Traits = petTraits;. You're passing a reference to the same list to all pets created by your function. There's 2 ways you can fix this, the easy way is to create a copy the list inside CreatePet() like pet.Traits = petTraits.ToList();

A better solution is to declare all of the temporary variables (kind, name, petTraits, etc.) inside your for loop. In C# you prefer to have variables that stick around for the shortest possible lifetime scope. Avoiding bugs like this is more important than some tiny performance edge from reusing variables.

  • Thankyou so much! yep that was exactly it, I've changed the variable name and moved it inside my loop as well to make sure! project is due tomorrow and I only just discovered the problem now so very much appreciate your timely help! – S.M Jun 04 '17 at 03:12