why there are duplicate values in list of circles in c#?

Question

I'm writing dijkstra algorithm ,i created a class called circle that it has property

public List<Circle> Circles = new List<Circle>();

but it values are duplicate it values initialize with each MouseDoubleClick . for example : properties values Name,Location and selected are duplicate. in event pictureBox1_MouseDoubleClick i save all of properties related to circle but As I said values are duplicate.i mean is this line circle.Circles.Add(circle);

public class Circle
{
    public List<Circle> Circles = new List<Circle>();
    public List<Rectangle> CircleShape = new List<Rectangle>();
    public string Name { get; set; }
    public Size size = new Size(25, 25);
    public Color normalFillColor = Color.White;
    public Color selectedFillColor = Color.Red;
    public Color borderColor = Color.Gray;
    public int borderWith = 2;
    // public int Diameter { get; set; }

    public Point Location { get; set; }
    public bool Selected { get; set; }

    public Rectangle Bounds
    {
        get
        {
            return new Rectangle(Location, size);
        }
    }

    public void HitTest(Point p)
    {
        //var result = false;
        for (int i = 0; i < CircleShape.Count; i++)
        {
            using (var path = new GraphicsPath())
            {
                path.AddEllipse(CircleShape[i]);
                if (path.IsVisible(p))
                {
                    Circles[i].Selected = true;
                }
            }
        }
    }

    private Font font = new Font("Tahoma", 8, FontStyle.Bold);

    public void Draw(Graphics g)
    {
        for (int i = 0; i < CircleShape.Count; i++)
        {
            using (var brush = new SolidBrush(Circles[i].Selected ? selectedFillColor : normalFillColor))
                g.FillEllipse(brush, CircleShape[i]);
            using (var pen = new Pen(borderColor, 2))
                g.DrawEllipse(pen, CircleShape[i]);
            TextRenderer.DrawText(g, i.ToString(), font,
                CircleShape[i], Color.Black,
                TextFormatFlags.VerticalCenter | TextFormatFlags.HorizontalCenter);
        }
    }
}

    namespace Dijkstra_s_algorithm
{
    public partial class Form1 : Form
    {
        private List<Rectangle> Shapes = new List<Rectangle>();
        private Circle circle = new Circle();
        int number = 0;
        public int Count { get { return number; } set { number = value; } }
        public Form1()
        {
            InitializeComponent();
            pictureBox1.Paint += new PaintEventHandler(pic_Paint);
        }

        private void pic_Paint(object sender, PaintEventArgs e)
        {
            e.Graphics.SmoothingMode = SmoothingMode.AntiAlias;
            Graphics g = e.Graphics;
            circle.Draw(g);
        }

        private void pictureBox1_MouseDoubleClick(object sender, MouseEventArgs e)
        {

            if (e.Button == MouseButtons.Left)
            {
                circle.Name = Count.ToString();
                Location.Offset(-circle.size.Width / 2, -circle.size.Height / 2);
                circle.Location = e.Location;
                circle.CircleShape.Add(new Rectangle(circle.Location, circle.size));
                pictureBox1.Invalidate();
            }
            circle.Circles.Add(circle);
            Count++;
        }



        private void pictureBox1_MouseClick(object sender, MouseEventArgs e)
        {
            if (ModifierKeys != Keys.Control && e.Button != MouseButtons.Right)
            {
                return;
            }
            else
            {

                circle.HitTest(e.Location);

            }

            pictureBox1.Invalidate();
        }
    }
}

Because you're not creating a new circle every time, you're just reusing the same object and changing its values. — itsme86, Aug 26 '16 at 15:13
You should not keep a `List` in `Circle` class. `List` belongs to your drawing surface or your form. Just change this [example](http://stackoverflow.com/a/38347945/3110834) to use `Circle`. Also for idea about how to use a `List` which belongs to drawing surface, take a look at ['Draw multiple freehand Polyline or Curve drawing'](http://stackoverflow.com/a/38297293/3110834) or ['How to drag and move shapes'](http://stackoverflow.com/a/38749134/3110834). You will find those examples useful. — Reza Aghaei, Aug 26 '16 at 18:52

score 2 · Accepted Answer · answered Aug 26 '16 at 18:14

Here's some example code that 1) creates a new circle each double-click (your original code used one circle object over and over), and 2) refactors the Circle class into CircleManager and Circle classes, so that you can separate logic dealing with the collection of circles while still having each circle be it's own individual object.

This compiled and ran fine for me, but you may still want to follow the code execution to see how the CircleManager is used.

using System.Windows.Forms;
using System.Collections.Generic;
using System.Drawing;
using System.Drawing.Drawing2D;

namespace CircleTest
{
    public partial class Form1 : Form
    {
        private CircleManager circleManager = new CircleManager();

        private Font font = new Font("Tahoma", 8, FontStyle.Bold);
        public Color normalFillColor = Color.White;
        public Color selectedFillColor = Color.Red;
        public Color borderColor = Color.Gray;
        public int borderWith = 2;

        public Form1()
        {
            InitializeComponent();
            pictureBox1.Paint += new PaintEventHandler(pic_Paint);
        }

        private void pic_Paint(object sender, PaintEventArgs e)
        {
            e.Graphics.SmoothingMode = SmoothingMode.AntiAlias;
            Graphics g = e.Graphics;
            DrawCircles(g);
        }

        public void DrawCircles(Graphics g)
        {
            for (int i = 0; i < circleManager.CircleShapes.Count; i++)
            {
                using (var brush = new SolidBrush(circleManager.Circles[i].Selected ? selectedFillColor : normalFillColor))
                    g.FillEllipse(brush, circleManager.CircleShapes[i]);
                using (var pen = new Pen(borderColor, 2))
                    g.DrawEllipse(pen, circleManager.CircleShapes[i]);
                TextRenderer.DrawText(g, i.ToString(), font,
                    circleManager.CircleShapes[i], Color.Black,
                    TextFormatFlags.VerticalCenter | TextFormatFlags.HorizontalCenter);
            }
        }

        private void pictureBox1_MouseDoubleClick(object sender, MouseEventArgs e)
        {
            if (e.Button == MouseButtons.Left)
            {
                Circle newCircle = new Circle();
                newCircle.Name = (circleManager.Circles.Count + 1).ToString();
                Location.Offset(-newCircle.size.Width / 2, -newCircle.size.Height / 2);
                newCircle.Location = e.Location;
                circleManager.Circles.Add(newCircle);
                circleManager.CircleShapes.Add(new Rectangle(newCircle.Location, newCircle.size));
                pictureBox1.Invalidate();
            }
        }

        private void pictureBox1_MouseClick(object sender, MouseEventArgs e)
        {
            if (ModifierKeys != Keys.Control && e.Button != MouseButtons.Right)
            {
                return;
            }
            else
            {
                circleManager.HitTest(e.Location);
            }
            pictureBox1.Invalidate();
        }
    }

    public class CircleManager
    {
        public List<Circle> Circles = new List<Circle>();
        public List<Rectangle> CircleShapes = new List<Rectangle>();

        public void HitTest(Point p)
        {
            for (int i = 0; i < CircleShapes.Count; i++)
            {
                using (var path = new GraphicsPath())
                {
                    path.AddEllipse(CircleShapes[i]);
                    if (path.IsVisible(p))
                    {
                        Circles[i].Selected = true;
                    }
                }
            }
        }
    }

    public class Circle
    {
        public string Name { get; set; }

        public Point Location { get; set; }
        public Size size = new Size(25, 25);
        public bool Selected { get; set; }

        public Rectangle Bounds
        {
            get
            {
                return new Rectangle(Location, size);
            }
        }

    }
}

thanks so much , i have a question,why don't you write drawcircles method into class circlemanager? — behnam, Aug 27 '16 at 04:47
You definitely can, and probably should - I was just focused on the separation of the two classes. If you want the CircleManager to handle it, you can just pass in the `Graphics` object (since the graphics is specifically related to the UI it is of course "owned" by the Form, but you can pass it in no problem). — SlimsGhost, Aug 27 '16 at 22:48

score 1 · Answer 2 · answered Aug 26 '16 at 15:17

1

Create new circle every time before adding it to list of circles :

    private void pictureBox1_MouseDoubleClick(object sender, MouseEventArgs e)
    {
        Circle newCircle = new Circle();

        if (e.Button == MouseButtons.Left)
        {
            circle.Name = Count.ToString();
            Location.Offset(-circle.size.Width / 2, -circle.size.Height / 2);
            circle.Location = e.Location;
            circle.CircleShape.Add(new Rectangle(circle.Location, circle.size));
            pictureBox1.Invalidate();
        }
        circle.Circles.Add(newCircle);
        Count++;
    }

answered Aug 26 '16 at 15:17

Umair M

10,298
6
42
74

if i want to create circle object with each MouseDoubleClick, but other events have a single circle object. for example :pic_Paint and pictureBox1_MouseClick , i'm testing but it doesn't work. – behnam Aug 26 '16 at 15:31
Well. In those handlers you need to choose one of the circle from list. – Umair M Aug 26 '16 at 15:38
3

it looks like you are using the `Circle` class for two different purposes. Consider having a container class named `CircleManager` or similar that contains the list of Circles, then separate the logic so that `Circle` only represents **ONE** circle, and `CircleManager` manages the list of circles. This will reduce the confusion a LOT. – SlimsGhost Aug 26 '16 at 15:45
I thought of this too, but this is an implementation of `Node -> AdjecentNodes` specifically used in path-finding/map-generation algorithms. OP got confused with it tho. – Umair M Aug 26 '16 at 15:49
@SlimsGhost,thanks for answer,i'm a novice programmer .Please write an example. – behnam Aug 26 '16 at 16:03

score 0 · Answer 3 · answered Aug 26 '16 at 15:13

0

You only have a single Circle object, which you change every time you add it to the list.

answered Aug 26 '16 at 15:13

Scott Hunter

48,888
12
60
101

if i want to create circle object with each MouseDoubleClick, but other events used of just one circle object. for example :pic_Paint and pictureBox1_MouseClick – behnam Aug 26 '16 at 15:20

why there are duplicate values in list of circles in c#?

3 Answers3