Problem with array list

[loftty]

Hi All,

I have this arraylist which I add 2 shape objects to it I also add the shape to a temp array list, why is it that when I change the values in a shape the values in the temp array list also get changed?

Highlighted red is where I add the shapes to the first temp array list
Highlighted blue is where I add the shapes to the second temp array list

Drawable shape;
        int ClickedShape = -1;
        ArrayList primitives = new ArrayList();
        List<ArrayList> Undo = new List<ArrayList>();
        int startx = 0;
        int starty = 0;
        bool CanMove = false;
        int UndoPosition = 0;
 
        private void button1_Click(object sender, EventArgs e)
        {
 
            GraphicsPath path = new GraphicsPath();
            Rectangle r = new Rectangle(10, 10, 45, 45);
            path.AddRectangle(r);
            primitives.Add(new Shape(path, Color.Red));
 
 
            GraphicsPath path2 = new GraphicsPath();
            Rectangle r2 = new Rectangle(50, 10, 45, 45);
            path2.AddRectangle(r2);
            primitives.Add(new Shape(path2, Color.Green));
 
            
            Draw();
 
            [COLOR=Red]ArrayList Temp = new ArrayList();
            foreach (Shape s in primitives)
            {
                Shape te = new Shape(s.GraphicsPath, s.Color);
                Temp.Add(te);
            }
            Undo.Add(Temp);[/COLOR]        }
 
        private void button2_Click(object sender, EventArgs e)
        {
            primitives.Reverse(ClickedShape, 2);
            Draw();
        }
 
        private void pane1_MouseMove(object sender, MouseEventArgs e)
        {
            if (CanMove == true)
            {
                shape = ((Drawable)(primitives[ClickedShape]));
                shape.Move(new Point(e.X, e.Y), new Point(startx, starty));
                Draw();
            }
        }
 
        private void pane1_MouseDown(object sender, MouseEventArgs e)
        {
            
            
            foreach (Shape s in primitives)
            {
                if(s.GraphicsPath.IsVisible(e.X,e.Y))
                {
                    CanMove = true;
                    ClickedShape = primitives.IndexOf(s);
                    shape = ((Drawable)(primitives[ClickedShape]));
                    
                }
            }
            startx = e.X - (int)shape.GraphicsPath.GetBounds().X;
            starty = e.Y - (int)shape.GraphicsPath.GetBounds().Y;
        }
 
        private void pane1_MouseUp(object sender, MouseEventArgs e)
        {
            CanMove = false;
            [COLOR=RoyalBlue]ArrayList Temp = new ArrayList();
            foreach (Shape s in primitives)
            {
                Shape te = new Shape(s.GraphicsPath, s.Color);
                Temp.Add(te);
            }
            
            Undo.Add(Temp);[/COLOR]
        }
 
        public void Draw()
        {
            Bitmap bat = new Bitmap(pane1.TempImage, pane1.Width, pane1.Height);
            Image d = bat;
            Graphics graphics = Graphics.FromImage(d);
            Drawable shapepy;
            for (int i = 0; i <= primitives.Count - 1; i++)
            {
                shapepy = ((Drawable)(primitives[i]));
                graphics.SmoothingMode = SmoothingMode.AntiAlias;
                graphics.FillPath(new SolidBrush(Color.FromArgb(255, shapepy.Color.R, shapepy.Color.G, shapepy.Color.B)), shapepy.GraphicsPath);
                graphics.DrawPath(new Pen(shapepy.Color, 2), shapepy.GraphicsPath);
            }
 
            pane1.Image = d;
        }
 
 
        private Image _Image()
        {
            Bitmap b = new Bitmap(pane1.Width, pane1.Height);
            Rectangle rect2 = new Rectangle(0, 0, pane1.Width, pane1.Height);
            pane1.DrawToBitmap(b, rect2);
 
            return b;
        }

[axion_sa]

The reference to the object is stored in the array list.
What you want to do is clone (use ICloneable or define your own interface - choosing either a shallow or deep clone - http://www.codeproject.com/dotnet/Clone.asp for more information), and add the cloned object to your temporary array list, for example:

Temp.Add(te.Clone());

[BramVandenbon]

yeah axion_sa is right. You should either clone your shape. Or you should clone the instances of the values of your shape's properties. I'll give an example of what I mean;

//let's say you have made a shape like this:
Graphicspath g = new GraphicsPath();
Color c = Color.Red;
Shape s = new Shape(g, c);

//now let's make a totally independant copy of this.
Shape s2 = s.Clone(); //axion_sa's suggestion.
// of course this will only work if the Clone() function exists. The Clone function is specified by the ICloneable interface.

//Another way to do it...
GraphicsPath g2 = s1.GraphicsPath.Clone(); //we make a copy first.
//here again the Clone() function will only be available if the ICloneable interface was implemented.
Shape s3 = new Shape(g2,s1.Color);

// Do you wonder why you should not clone the color just like 
// we cloned the GraphicsPath??? That's because Color is not a
// class, Color is a structure! Structures are value-types, not
// reference-types. (Same goes for integers.)

//Now, what if the clone function is not available and you don't want to
// implement it. Then you can also do it like this:
GraphicsPath g3 = new GraphicsPath();
foreach(object o in GraphicsPath){
    g3.Add(o);
    //why not g3.Add(o.Clone()) ? because in your codelisting
    //you showed you used Rectangles. Rectangles are structures,
    // not classes ;-). No need to clone them in other words.
}
Shape s4 = new Shape(g3,s1.Color);

//What if you don't clone anything at all. It won't work because ...
Shape s5 = new Shape(s1.GraphicsPath, s1.Color);
if (s5.GraphicsPath == s1.GraphicsPath){
    Console.WriteLine("This will be true, both GraphicsPath's point to the same instance.");
}
s1.GraphicsPath.Add(Whatever);
//s5.GraphicsPath is the same GraphicsPath, so it will also be added there.

[loftty]

Hi,

Thanks for your help using IClonable has worked

Thanks

Loftty