-1

I'll try to be as brief as possible.

I have a class named Error. This class contains a property named Suggestions which is an ObservableCollection<string>.

In one method, I create a list of Errors named AllInstances with a LINQ statement and then do a foreach loop on that list. Inside the loop, I add one item to the Suggestions collection of the Error object.

The problem is that on each turn of the loop, the item is added to ALL of the members of the AllInstances. Therefore, if the loop has 3 turns, then every member of the AllInstances will have 3 items added to them!

The normal behavior should be that each Suggestions collection have 1 item added to it, not 3.

Here's the code:

Error Class

public class Error : INotifyPropertyChanged, ICloneable
    {
        public ObservableCollection<string> Suggestions { get; set; }
    }

The Method's Code

List<Error> AllInstances = (from paragraph in DocRef.LatestState
                            from error in paragraph.Errors
                            where error.Word == sourceError.Word
                            select error).ToList();

foreach (Error _er in AllInstances)
    {
        _er.Suggestions.Add(newSuggestion);
    }

// At this point, if "AllInstances.Count" was 3, every "_er.Suggestions" in "AllInstances"
// will also have 3 items in it!

This seems so illogical to me that I highly suspect it's something that I've done or something basic about the structure of types that I don't yet know.

I appreciate any help. Thank you.

DreamWalker
  • 19
  • 4
  • is newSuggestion object a list? – Sachin Vishwakarma Jul 31 '19 at 11:42
  • @SachinVishwakarma No, "newSuggestion" is just a string, as `Suggestions` collection itself is of type ``. – DreamWalker Jul 31 '19 at 11:47
  • Are you sure each Error has it's own Suggestions? Seems that it is the same collection for each Error. – Pablo notPicasso Jul 31 '19 at 11:58
  • @PablonotPicasso I can be as sure as my code says unfortunately. It "seems" that they are one and the same but the code is working fine in every other part of the program. This foreach loop is the only place that's caused an error so far. – DreamWalker Jul 31 '19 at 12:02
  • Could you check that your LINQ query does not return three times the same Error instance ? It could be the cause, if multiples paragraphs share error instances. – Antoine Jul 31 '19 at 12:26
  • @Antoine Thank you. Yes, I've made sure of that, everything is separate and Error objects are not the same. As I've put in the code, `Error` class inherits from `IClonable` and I looked over the cloning part. It seems that the clone is working fine for non-collection properties of the class but does not work as it should for the `Suggestions` collection. Each `Error` object is different in the loop but somehow, cloning the only collection field, results in all of them referencing the same object. I'm trying to look into the reason. – DreamWalker Jul 31 '19 at 12:52

1 Answers1

1

Okay, so as I mentioned in my last comment on the question, I looked into the way I was cloning the Error class in a few instances where I needed it. Turns out I was cloning the object like this in the method:

public object Clone ()
        {
            return new Error
            {
                ...
                Suggestions = Suggestions,
                ...
            };
        }

I noticed that every other property of Error class is a default type (immutable?) like string, int and bool, and the only different one is the Suggestions collection of type ObservableCollection<string>. I figured that maybe because the other properties are all behaving correctly and have not caused problems of referencing to the same instance, this collection is, by design, different from the others and needs to be re-instantiated manually. And so I did:

public object Clone ()
        {
            return new Error
            {
                ...
                Suggestions = new ObservableCollection<string>(Suggestions),
                ...
            };
        }

I simply re-created the property in the Clone method and the problem of referencing to the same instance in the foreach loop was solved.

I hope someone with deeper knowledge of the code can enlighten me and others who may stumble upon this, about the logic of this problem and the difference between those immutable types and an ObservableCollection.

DreamWalker
  • 19
  • 4
  • An `ObservableCollection` is a mutable reference type. If you don't clone it, you now have two references pointing to the same collection. Think of it like a box. If you are looking at the box, and add something to it - you see it. Now I start looking at the box. When you add something to it I see it too. This is because **we are both looking at the same box**. Using `new ObservableCollection` works because, instead, you go and get a new box and put in it a copy of all of the things in it. Now you are looking at one box, and I am looking at a **separate box**. – mjwills Aug 05 '19 at 10:43
  • @mjwills Thank you for your simple and precise explanation. It seems that my guess was correct but I just wasn't aware of the types that are and are not immutable. Now that you have cleared it up, it is indeed a duplicate question, sorry! – DreamWalker Aug 06 '19 at 08:27