Resolved List Containing Multiple Instances of the Same Object

[Madaxe]

Something Odd is happening, even though I created a new object to add to the list in the using statement, it seems to be self referencing, the second time it loops through its adding the same object even though i created a new one and the objects inside are compounding.

So at the end i have 7 identical XMLMachiningModel's in the list each containing 42 objects in the Layout, it should be 7 different XMLMachiningModel's each containing 6 Objects in the Layouts

I have no idea why, can anybody suggest a solution or a path to investigate

Thanks

Madaxe

        public MemoryStream GetReaJetXMLZips(IEnumerable<JsonRequestModel> JsonRequestModels=null)
        {
            MemoryStream ReturnMemoryStream = null;

            //For Testing Locally Only
            if (JsonRequestModels == null){JsonRequestModels = this.GetTestXMLRequest();}

            foreach(JsonRequestModel JsonRequestModel in JsonRequestModels)
            {
                // Build MySQL In List
                string MySQLInPartNumbers = string.Concat(String.Join("','", JsonRequestModel.slatPartNumbers.Select(i => i.ToString()).ToArray()));
               
                // Build The XML Structures
                IEnumerable<SlatModel> SlatModel = this._NewRepo.GetSlatModels(MySQLInPartNumbers);

                using (XMLMachiningModel NewXMLMachiningModel = this.BuildSlatsXML(SlatModel, MySQLInPartNumbers))
                {
                    this.UpdateOneOffObjects(NewXMLMachiningModel);
                    this.BuildConnectionObjects(NewXMLMachiningModel, MySQLInPartNumbers);

                    this._XMLMachiningModels.Add(NewXMLMachiningModel);
                }
            }

            // Serialize the XMLs and Add to Zip Files
            ReturnMemoryStream = this.BuildRootZip();
            ReturnMemoryStream.Seek(0, SeekOrigin.Begin);

            return ReturnMemoryStream;
        }
        private XMLMachiningModel BuildSlatsXML(IEnumerable<SlatModel> iSlatModelsList, string iMySQLInPartNumbers)
        {
            XMLMachiningModel ReturnXMLMachiningModel = new XMLMachiningModel(this._NewRepo, iSlatModelsList);

            ReturnXMLMachiningModel.ReaJetSideBase = BuildReaJetSideBaseXMLMachiningModel(iSlatModelsList, iMySQLInPartNumbers);
            ReturnXMLMachiningModel.ReaJetTopLeftBase = BuildReaJetTopLeftBaseXMLMachiningModel(iSlatModelsList, iMySQLInPartNumbers);
            ReturnXMLMachiningModel.ReaJetTopRightBase = BuildReaJetTopRightBaseXMLMachiningModel(iSlatModelsList, iMySQLInPartNumbers);

            ReturnXMLMachiningModel.SetMachiningLength(iSlatModelsList, iMySQLInPartNumbers);

            return ReturnXMLMachiningModel;
        }

 

[Skydiver]

Not directly related to your compounding object problem above, but Just like your other thread where you are disposing your memory stream object and then giving it away, in this case you are putting in a reference to your object into your _XMLMachiningModels collection (line 21) and then disposing of that object as you exit the scope on line 22.

 

[Madaxe]

I tried with and without the using statement, I'm coming from a vba background, and the rules seems to be a little different.

I'm going to rewrite it step by step, I appreciate the help

Madaxe

 

[Madaxe]

I think this is my problem, I have XML templates stored in the database, I read these in and serialize them to create a template which is stored in my implementation as a private property.
When i need a copy I call the Clone method. I suspect its maintaining a reference back to the private property.

In Fact that's exactly what's it's doing how can i clone an object and make it a new non-instance. The image below is the template that i thought i was cloning and it has the same number of objects as the duplicate sin the list.

REAJET ReturnReaJet = (REAJET)this._ReaJetSideBase.Clone();

namespace InfrastructureProject.Models.XMLModels
{
    [XmlRoot(ElementName = "REA-JET")]
    public class REAJET : ICloneable
    {
        public object Clone()
        {
            return (REAJET)this.MemberwiseClone();
        }

        [XmlElement(ElementName = "Label")]
        public Label Label { get; set; }
    }
}

 

[Skydiver]

Are you sue you want to that? Read the documentation of MemberwiseClone():

Creates a shallow copy of the current Object.

(emphasis mine)

 

[Skydiver]

I'm coming from a vba background, and the rules seems to be a little different.

VBA and C# have the same object reference rules. In VBA, when the last reference is released, the object is destroyed. In C#, when the last reference is released, the object is queued for garbage disposal.

I suspect that you maybe conflating the VB/VBA WITH keyword and the C# using keyword.

 

[Madaxe]

I found a fix, by serializing and deserializing my template, surely there is a better way

        private T DeepClone<T>(T obj)
        {
            using (var ms = new MemoryStream())
            {
                var formatter = new BinaryFormatter();
                formatter.Serialize(ms, obj);
                ms.Position = 0;

                return (T)formatter.Deserialize(ms);
            }
        }

 

[Skydiver]

I know I commented in your other thread regarding your naming convention. I'll reiterate. Unless you work in a team that specifically uses that current naming convention that you are currently using, you should try to follow the .NET Framework naming conventions:

Naming Guidelines - Framework Design Guidelines

In this overview, read about naming conventions to use in framework development. Go to articles covering capitalization, general naming, and other guidelines.

docs.microsoft.com

I will admit, though, that for private members, I don't follow the guideline that says not to use Hungarian. I use the underscore Hungarian prefix to denote private fields like you do, but I still follow the Camel casing recommended by the guidelines. Furthermore, I turn off the StyleCop rule (as well as the ReSharper rule) that suggests using the this in front of the field name. This becomes redundant if you follow the rule:

this._newRepo

If you use the Hungarian, then just have:

_newRepo

Or if you follow the StyleCop/ReSharper rule, then just have:

this.newRepo

Don't use both.

 

[Skydiver]

I found a fix, by serializing and deserializing my template, surely there is a better way

The best way to to implement your deep cloning by hand. Yes, it becomes tedious boilerplate code where you have to call DeepClone() on each of your fields, and each of your field classes have to implement DeepClone().

The other options are to use reflection, but it's even more complex than using that serialize/deserialize trick you are doing there. Under the covers, the serializers use reflection. If you continue to use serialize/deserialize, then any speed gains that you may have gained by loading your prototype into memory is lost due to the cost of serializing and deserializing. If you use reflection, it'll be slightly faster than serialization/deserialization since you don't pay the price of writing/reading to the serialization memory stream.

So it becomes your choice: Is developer time is more expensive than user time, or is user time more expensive that developer time? Do you spend 4 hours writing deep cloning by hand, or do you make your users pay 2 seconds of time every time they use your code from now until eternity? Realize that if you those 2 seconds add up -- If you have 100 users, they would just have to use your app 72 times for the 2 seconds to be equivalent to your 4 hours. 4 hrs * 60 minutes/hr * 60 seconds/min == 14400 seconds == 72 runs * 100 users * 2 seconds/user = 14400 seconds.

 

[Madaxe]

Thanks for the Feed back, I just write prototypes, fur my proof of concepts and hand it off to my dev team with requirements. They do the design doc and real code.

I just play, butI will endeavor to do what you said.
Thanks
Mad axe