2

The text variable is unintentionally modified by the InsertWords() function. If the variable text.text is "example ˣ here", with the variable text.words[0] being "TEST", the InsertWords() function will change the text.text to "example TEST here". I want the text variable to remain unchanged, while only the textCopy variable changes. How would I do that? And why does text.text change even though I never use regex.Replace on it?

public class TextClass {

    public List<string> words;
    public string text;
    public string name;
    public string date;
    public string id;

}

public TextClass text;

public TextClass InsertWords()
    {
        Regex regex = new Regex(Regex.Escape("ˣ"));

        TextClass textCopy = text;

        foreach (string word in inputWords)
        {
            textCopy.text = regex.Replace(textCopy.text, word, 1);
        }

        return textCopy;
    }

Edit: I use the function like this

public Display display;

display.DisplayText(InsertWords());

public class Display {

    public Text text;

    public void DisplayText (TextClass text_)
    {
        text.text = text_.text;
    }

}
Wiktor Stribiżew
  • 607,720
  • 39
  • 448
  • 563
Asgeir
  • 593
  • 6
  • 21
  • `textCopy` is a reference to the same object as `text`. If you want to copy it you need to create a new `TextClass` and copy values over. – juharr Jan 26 '20 at 15:56
  • @juharr Do I need to do textCopy.text = text.text for all the classes attributes? – Asgeir Jan 26 '20 at 15:57
  • 1
    Yes, tough you could create a constructor on `TextClass` that takes a `TextClass` and copies the values over. Note you'd need to create a new list for `words` and fill it up if you want a deep copy that would allow you to change the items in `words` for `textCopy` without it effecting the list in `text`. – juharr Jan 26 '20 at 15:59
  • Also I'd suggest changing from public fields to auto properties like `public string Text {get;set;}` – juharr Jan 26 '20 at 16:00
  • Not really a RegEx issue. Probably a duplicate of https://stackoverflow.com/questions/78536/deep-cloning-objects – Ian Mercer Jan 26 '20 at 18:44

2 Answers2

3

In order to have a copy of a class you need to create a new instance of that class. You can do that with a constructor like this. Also I changed all your fields to properties.

public class TextClass
{
    // You don't need to set the list externally, just get it to add/remove/iterate
    public List<string> Words { get; } = new List<string>();
    public string Text { get; set; }
    public string Name { get; set; }
    public string Date { get; set; }
    public string Id { get; set; }

    public TestClass() { } // default constructor

    public TestClass(TestClass from) // copy constructor
    {
        Text = from.Text;
        Name = from.Name;
        Date = from.Date;
        Id = from.Id;
        Words.AddRange(from.Words); // this ensures a deep copy of the list
    }    
}

Then you can do

TextClass textCopy = new TextClass(text);

And textCopy will be a true deep copy of text and when you assign something to textCopy.Text and it will not effect text.Text.

Alternatively you can make a copy method like this

public class TextClass
{
    // You don't need to set the list externally, just get it to add/remove/iterate
    public List<string> Words { get; } = new List<string>();
    public string Text { get; set; }
    public string Name { get; set; }
    public string Date { get; set; }
    public string Id { get; set; }

    public TestClass Copy()
    {
        var copy = new TestClass()
        {
            Text = this.Text;
            Name = this.Name;
            Date = this.Date;
            Id = this.Id;
        }
        copy.Words.AddRange(this.Words); // this ensures a deep copy of the list
        return copy;
    }    
}

And use it like this

TextClass textCopy = text.Copy();
juharr
  • 31,741
  • 4
  • 58
  • 93
1

An alternative more 'fluent' way to do this is to create methods returning new instances that are explicit about what is changing. This will allow you to make the whole object immutable and will allow maximum reuse of nested objects that are not changing, it's also easier for someone else to understand.

There is no need to clone the Words property for example:

public class TextClass
{
    // No mutable properties (ideally replace List with IReadonlyCollection too)
    public List<string> Words { get; }
    public string Text { get; }
    public string Name { get; }
    public string Date { get; }
    public string Id { get; }

    public TestClass (List<string> words, string text, string name, string date, string id)
    {
       this.Words = words ?? throw new ArgumentNullException(nameof(words));
       ...
    }

    public TestClass WithNewText(string text) =>
       new TestClass(this.Words, text, this.Name, this.Date, this.Id);
 }
Ian Mercer
  • 38,490
  • 8
  • 97
  • 133