RemoveAt() should not delete values in multiple C#-lists-CodePudding

I created a program with multiple classes and multiple lists in each class. In the following you'll find a code example:

RsiClass rsi = new RsiClass();
RSI = await rsi.RsiDrawGraph(valueList);

The function rsi.RsiDrawGraph() (in my child class) changes the values in the list:

public async Task<List<RsiModel>> RsiDrawGraph(List<ChartDataModel> allValue)
{
    List<RsiModel> returnValues = new List<RsiModel>();
    for (int i= allValue.Count - 1; i>=0; i--)
    {
        returnValues.Add(new RsiModel { LowerBorder = 30, UpperBorder = 70, rsi = await Rsi(allValue), Time = allValue[i].Time, x=allValue[i].x });
        allValue.RemoveAt(i);
    }
    return returnValues;
}

Finally, the function returns the expected results but the values in my other "valueList" also changed too. In detail, it seems that the removeAt() command effected my other valueList in the main class, because the valueList is empty after the operation.

Why does it happened and how can I prevent, that the childclass is effecting my list in the main class?

CodePudding user response：

Let's assume the caller of RsiDrawGraph does not expect you to modify allValue you can do something like this:

public async Task<List<RsiModel>> RsiDrawGraph(IEnumerable<ChartDataModel> allValue)
{
    List<ChartDataModel> chartData = new List<ChartDataModel>(allValue);
    List<RsiModel> returnValues = new List<RsiModel>();
    for (int i = chartData.Count - 1; i > = 0; i--)
    {
        returnValues.Add(new RsiModel { LowerBorder = 30, UpperBorder = 70, rsi = await Rsi(chartData), Time = chartData[i].Time, x=chartData[i].x });
        chartData.RemoveAt(i);
    }
    return returnValues;
}

This way, you're modifying a copy of allValue that is local to the method so you avoid unexpected side effects.

CodePudding user response：

You can not delete from a collection while iterating over it. you should make a copy of it by using .ToList(). Moreover, list of objects is passed by reference in the function parameter. That's why your other "valueList" also have same effect as allValue. Here allValue & valueList is pointing to the same memory location. make a copy of allValue so that valueList won't change

public async Task<List<RsiModel>> RsiDrawGraph(List<ChartDataModel> allValue)
{
    List<ChartDataModel> allValueCopy = allValue.ToList();
    List<RsiModel> returnValues = new List<RsiModel>();
    for (int i= allValueCopy.Count - 1; i>=0; i--)
    {
        returnValues.Add(new RsiModel { LowerBorder = 30, UpperBorder = 70, rsi = await Rsi(allValueCopy), Time = allValueCopy[i].Time, x=allValueCopy[i].x });
        allValueCopy.RemoveAt(i);
    }
    return returnValues;
}

CodePudding user response：

You use allValue list in for loop and you can't remove items in list when you use list Count property in for loop.

CodePudding user response：

When I look at the code, it strikes me that you should not affect the input list... only return a list.. so I reckon something like this is the better solution. Altering an input parameter is not really good practice.

    public async Task<List<RsiModel>> RsiDrawGraph(List<ChartDataModel> allValue)
{
    return allValue.OrderDescending(l => allValue.IndexOf(l)).Select( v => new RsiModel { LowerBorder = 30, UpperBorder = 70, rsi = await Rsi(v), Time = v.Time, x=v.x }).ToList();
}