Why is my data changing when I am not changing it?

Question

I am working on a Microsoft Visual C# Form Application that is to be used to create all the data that would go into RPG game. I have designed a structure and encased it all into a class so I can easily read and write to/from an XML file.

On my main form I have a "public static"/"Global" variable that all of my sub forms can copy information from... manipulate what it needs to... and send that information back to it.

For example. I want multiple types of currencies. The form that deals with currencies copies only the currency data from the global variable and has free reign to manipulate only THAT copy. Only when the user clicks the "Apply" or "Accept" button should the global variable be updated to reflect those changes. If the "Cancel" button is clicked it should just close the form and the copied data should get tossed to the winds when the form is disposed.

Unfortunately this is not the case. Whenever I change the data of the copy its changes seem to reflect on the global variable as well. Is there a concept here I am missing here and don't understand. Somebody please explain. I've checked over my code and there are only those two points where the data should be updated.

Code from "Main" Form

public partial class frmMain : Form
{
    public static RPGDataCollection DATA = new RPGDataCollection();
    public static string FILE = "";

    public frmMain ()
    {
        InitializeComponent();
        FillDefaultData();
    }

    /// <summary>
    /// Sets item info status text.
    /// </summary>
    /// <param name="text">Text to be displayed.</param>
    private void SetItemInfo (string text)
    {
        lblItemInfo.Text = text;
    }

Currency Form Code

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;

using RPGData.ObjectTypes.DataGroups;

namespace RPGData.EntryForms
{
    public partial class frmCurrencies : Form
    {
        #region Fields

        private List<Currency> datCurrencies = new List<Currency>();
        private string strEntry = "";
        private bool blnGSC = false;
        private bool blnUpperFlag = false;
        private int intIndex = 0;

        #endregion

        #region Constructor

        public frmCurrencies ()
        {
            InitializeComponent();
        }

        #endregion

        #region Events

        private void frmCurrencies_Load (object sender, EventArgs e)
        {
            datCurrencies = frmMain.DATA.Currencies;
            DisplayData();
        }

        private void btnReplace_Click (object sender, EventArgs e)
        {
            intIndex = lstCurrencies.SelectedIndex;
            Currency c = datCurrencies[intIndex];

            if (txtEntry.Text.Trim().Length > 0)
            {
                SetValues();

                c.Name = strEntry;
                c.HandlesGSC = blnGSC;
                c.Amount = 0;

                datCurrencies[intIndex] = c;
            }

            ResetFields();
            DisplayData();
        }

        private void btnCancel_Click (object sender, EventArgs e)
        {
            this.Close();
        }

        private void btnAdd_Click (object sender, EventArgs e)
        {
            if (txtEntry.Text.Trim().Length > 0)
            {
                SetValues();

                Currency c = new Currency();
                c.Name = strEntry;
                c.HandlesGSC = blnGSC;
                c.Amount = 0;

                datCurrencies.Add(c);
            }

            ResetFields();
            DisplayData();
        }

        private void btnRemove_Click (object sender, EventArgs e)
        {
            intIndex = lstCurrencies.SelectedIndex;

            if (intIndex >= 0)
                datCurrencies.RemoveAt(intIndex);

            ResetFields();
            DisplayData();
        }

        private void btnApply_Click (object sender, EventArgs e)
        {
            frmMain.DATA.Currencies = datCurrencies;
        }

        private void btnAccept_Click (object sender, EventArgs e)
        {
            frmMain.DATA.Currencies = datCurrencies;
            this.Close();
        }

        private void lstCurrencies_SelectedIndexChanged (object sender, EventArgs e)
        {
            intIndex = lstCurrencies.SelectedIndex;
            Currency c = datCurrencies[intIndex];

            txtEntry.Text = c.Name;
            chkGSC.Checked = c.HandlesGSC;
        }

        #endregion

        #region Methods

        private void DisplayData ()
        {
            lstCurrencies.Items.Clear();

            for (int i = 0; i < datCurrencies.Count; i++)
            {
                string gsc = "";
                if (datCurrencies[i].HandlesGSC)
                    gsc = "*";
                else
                    gsc = " ";

                lstCurrencies.Items.Add("[ " + gsc + " ] " + datCurrencies[i].Name);
            }
        }

        private void ResetFields ()
        {
            strEntry = "";
            blnGSC = false;

            txtEntry.Text = strEntry;
            chkGSC.Checked = blnGSC;

            txtEntry.Focus();
        }

        private void SetValues ()
        {
            string entry = ToAllUpper(txtEntry.Text);

            strEntry = entry;
            blnGSC = chkGSC.Checked;
        }

        private string ToAllUpper (string str)
        {
            string entry = "";

            for (int i = 0; i < str.Length; i++)
            {
                string c = "";

                if (i == 0)
                    c = str.Substring(0, 1).ToUpper();

                else if (str.Substring(i, 1) == " ")
                {
                    c = " ";
                    blnUpperFlag = true;
                }

                else if (blnUpperFlag)
                {
                    c = str.Substring(i, 1).ToUpper();
                    blnUpperFlag = false;
                }
                else
                    c = str.Substring(i, 1);

                entry += c;
            }

            return entry;
        }

        #endregion
    }
}

Any help you can toss me and help me understand what might be happening would be great (or you see a bug or mistake I don't).

Thanks!

Need to see the code for RPGDataCollection, but in any case, your design is formalising the need for a shared mutable state, at the end of which path madness lies. Don't use globally static references to access data; the problem that you're hitting now is but the tip of the iceberg. — Matt, Jan 26 '15 at 15:16
@ Matt : Is there another method to contain everything I need in a neat little package and have it accessible to every form needed to manipulate the data within? The data structure for housing everything contains probably close to 100 nested classes. I'm open for suggestions on that front. RPGDataCollection is not finished as far as the XML parsing portion. but it does enscapsulate the entire structure. — Robert Fleck, Jan 26 '15 at 16:28

Andrey Korneyev · Accepted Answer · 2015-01-26T15:32:30.927

2

This line of code datCurrencies = frmMain.DATA.Currencies is actually creates another one reference to frmMain.DATA.Currencies and doesn't deep copying it.

So all changes you made - is actually made on original object.

You have to clone it (create deep copy), not just create additional reference.

For example, if your Currency is struct (value-type), following will be sufficient:

datCurrencies = new List<Currency>(frmMain.DATA.Currencies);

But if your Currency is class - you may consider following approach: create Clone method in your Currency class that will return clone of current object and then populate your datCurrencies like:

datCurrencies = new List<Currency>(frmMain.DATA.Currencies.Count);
foreach(var currency in frmMain.DATA.Currencies)
    datCurrencies.Add(currency.Clone());

edited Jan 26 '15 at 15:32

answered Jan 26 '15 at 15:16

Andrey Korneyev

26,353
15
70
71

This will still edit the currencies within the list, unless they are structs. – Rawling Jan 26 '15 at 15:22
@Rawling oh, you're right. I've updated the answer. – Andrey Korneyev Jan 26 '15 at 15:33
@AndyKorneyev : Read it over a few more times. I believe I understand now and that's the nature of static variables/references. Looks like I got more work on this than I thought. Anyway... thank you! – Robert Fleck Jan 26 '15 at 16:33

score 1 · Answer 2 · answered Jan 26 '15 at 15:14

You copy the reference to a list here:

datCurrencies = frmMain.DATA.Currencies;

Afetr this asignment there is still just one list and datCurrencies points to this one global list.

Instead you need to create a deep copy i.e copy the contents from your global list to your local list.

Why is my data changing when I am not changing it?

Code from "Main" Form

Currency Form Code

2 Answers2