Search code examples
c#listforeachilistreference-type

Unwanted list modification


I have an issue with trying to modify a list of Transactions within a foreach. I have created copies of the list passed into my method, made it read only, and yet when I try to change a value within any of the lists it changes that value within them all. Some type of memory link? I am unsure how to resolve this issue.

My program starts by declaring a class called Transaction (which is a generic class having Name, Value, Formatting), then I have subClasses :Transaction. I create a TransList (from public class TransList : IEnumerable) which has an object instance of each subClass. So a TransList would include a class named TranID, Amount, OrderID, Time, CardType, Comment1, Comment2. Each value of these subclasses could be string, decimal, DateTime. After that a list of TransParts is created it is then put into a larger list called processTrans.

So Comment2 is the element with a payment citation number and if there is more than one number in there I want to separate that into more than one TransList add those new TransLists to processTrans and remove the non-separated one. From my code below, having tried every strategy, run-time modification occurs to not only the intended processTrans but also to tempProcessTrans, addOn, tran, tranPart.

If processTrans that is passed into method looks like this in the debugger locals
processTrans [0] _Items TranID.Value = SD234DF and Comment2 = adf;wer;
Then the output should be
processTrans [0] _Items TranID.Value = SD234DF-1 and Comment2.Value=adf
processTrans [1] _Items TranID.Value = SD234DF-2 and Comment2.Value=wer
I currently get
processTrans [0] _Items TranID.Value = SD234DF-1-2 and Comment2.Value=wer
processTrans [1] _Items TranID.Value = SD234DF-1-2 and Comment2.Value=wer

public static List<TransList> SeperateMultiCitations(List<TransList> processTrans) //change TransList seperating Multiple Citations
    {
        List<int> indexes=new List<int>();
        IList<TransList> tempProcessTrans = processTrans.AsReadOnly(); //this didn't help
        List<TransList> addOn= new List<TransList>(); //copy list didn't stop from changes to occur in processTrans at same time
        foreach (TransList tran in tempProcessTrans.ToList())
        {
            TransList copyTransList = tran;
            foreach (Transaction tranPart in tran.OfType<Comment2>())
            {
                if (new Regex(";.+;").IsMatch((string)tranPart.Value, 0))
                {
                    string[] citations = Regex.Split((string)tranPart.Value, ";").Where(s => s != String.Empty).ToArray();
                    int citNumb = 1;
                    indexes.Add(tempProcessTrans.IndexOf(tran));

                    foreach (string singleCitation in citations)
                    {
                        addOn.Add(ChangeTrans(tran, singleCitation, citNumb++)); when this line runs changes occur to all lists as well as trans, tranPart
                    }
                    break;
                }
            }
        }
        foreach (int index in indexes.OrderByDescending(x => x))
        {
            processTrans.RemoveAt(index);
        }
        processTrans.AddRange(addOn);
        return processTrans;
    }
public static TransList ChangeTrans(TransList copyTransList, string singleCitation, int citNumb) //add ConFee
    {
        foreach (Transaction temp in copyTransList.OfType<TranID>())
        {
            temp.Value += "-" + citNumb;
        }
        foreach(Transaction temp in copyTransList.OfType<Comment2>())
        {
            temp.Value = singleCitation;
        }
        foreach (Transaction temp in copyTransList.OfType<Amount>())
        {
            //temp.Value = DboGrab(temp);
            //temp.Value = amount;
        }

        return copyTransList;
    }

public class Transaction : TranInterface
{
    public string Name;
    public object Value;
    public string Formating;

    public Transaction(string name, object value, string formating)
    {
        Name = name;
        Value = value;
        Formating = formating;
    }
 }

 class TranID : Transaction
      {
          public TranID(string Name, string Value, string Formating) : base("Transaction ID",  Value, "@") { }
      }
 public class TransList : IEnumerable<Transaction> //not to add all the lengthy parts here but this just allows for adding the parts and iterating through them in the foreach statements
 {}

Solution

  • The behavior you're seeing is an inherent feature of reference types. When you call the ChangeTrans() method, the reference returned by that method is exactly the same as the one you passed in, which is that original value tran. Within the inner loop, the value of tran never changes, so on each iteration of the loop, you are modifying the same object over and over, adding it to your addOn list with each iteration.

    This has two undesirable effects:

    1. There is no difference between each element in the addOn list. They are all identical, referencing the same single object.
    2. Any modification of any single element in the addOn list, or via the original reference to that single object, is visible via every other reference to that same single object. I.e. via all of the other elements in the list, and even that original reference in the tran variable (and of course, the copyTranList variable, which was assigned to the value of tran).

    Without a more complete code example, it's not possible to know for sure what the best solution would be. However, one naïve solution would be to simply change your ChangeTrans() method so that it is responsible for making the new copy:

    public static TransList ChangeTrans(
        TransList copyTransList, string singleCitation, int citNumb) //add ConFee
    {
        TransList newTransList = new TransList();
    
        foreach (Transaction temp in copyTransList.OfType<TranID>())
        {
            Transaction newTransaction = new TranID();
    
            newTransaction.Value = temp.Value + "-" + citNumb;
            newTransList.Add(newTransaction);
        }
        foreach(Transaction temp in copyTransList.OfType<Comment2>())
        {
            Transaction newTransaction = new Comment2();
    
            newTransaction.Value = singleCitation;
            newTransList.Add(newTransaction);
        }
    
        return newTransList;
    }
    

    Note: I have no idea if the above actually would compile, or if it actually copies all of the values needed. I reiterate: since you have not shown the TransList or Transaction data structures, it's not possible to know what all in them needs to be copied, nor what the best way to copy those values would be.

    That said, note in the above example that this version of the method:

    1. Creates an entirely new instance of the TransList object, storing the reference in newTransList.
    2. For each Transaction value to be modified, it creates an entirely new instance of Transaction (using the appropriate type), assigning to that instance's Value property the modified value.
    3. For each of those new Transaction objects, it adds the object to the newly-created TransList object referenced by the newTransList variable.
    4. Finally, it returns that newly-created TransList object, rather than the one that was passed to the method.

    Presumably you know what the correct way to add Transaction elements to TransList object, as well as whether there are other members in a Transaction object that would need to be copied. The above is simply a basic illustration of where and how you can modify your code so that you do the "deep copy" needed to avoid the problem you're describing.