0

I would like to know why my templist.clear() method clears the list I added to my ManhattanDistance Dictionary.

Any help in this regard would be much appreciated, this is part of my data mining project which I have been working on. I have to impute the missing values using k nearest neighbor approach.

public void CalculateManhattanDistance(Dictionary<int, List<string>> MissingList, Dictionary<int, List<string>> OtherList)
{
    Dictionary<int,Array> MissingListNeighbours = new Dictionary<int,Array>();
    Dictionary<int, List<int>> ManhattanDistanceList = new Dictionary<int,List<int>>();
    List<int> tempList = new List<int>();

    int total=0;
    int k=0;

    try
    {
        for (int i = 0; i < MissingList.Count(); i++)
        {
            for (int j = 0; j < OtherList.Count(); j++)
            {
                for (k = 0; k < MissingList[0].ToArray().Length; k++)
                {
                    if (Convert.ToChar(MissingList[i][k].ToString()) == '?')
                        continue;
                    else
                        total += Math.Abs(Convert.ToInt32(MissingList[i][k].ToString()) - Convert.ToInt32(OtherList[j][k].ToString()));
                }
                tempList.Add(total);

                total = 0;
            }
            ManhattanDistanceList.Add(i, tempList);

            tempList.Clear();
        }
    }
    catch (Exception ex)
    {
          ex.Message.ToString();
    }
}
CodeCaster
  • 147,647
  • 23
  • 218
  • 272
  • It was some browser bug, i'm afraid, I didn't replace all the `<` and `>`, sorry about that. @CodeCaster – enb081 Apr 26 '13 at 12:03
  • 1
    @enb081 it's not your problem, it's what the review system is for. If you see any more issues please make another edit. :) – CodeCaster Apr 26 '13 at 12:04

2 Answers2

6

Because ManhattanDistanceList.Add(i, tempList); adds a reference to the same list tempList is pointing to, so when you later clear the list tempList is pointing to, ManhattanDistanceList[i] also gets cleared.

Change it to ManhattanDistanceList.Add(i, tempList.ToList()); to add a copy of the list.

CodeCaster
  • 147,647
  • 23
  • 218
  • 272
  • +1, I deleted mine, the use of `ToList()` is a lot better than building a new `List(tempList)`. – Mike Perrenoud Apr 26 '13 at 12:04
  • @MichaelPerrenoud thanks. However `.ToList()` [seems](http://stackoverflow.com/a/2774145/266143) to just return `new List(source);`. :) – CodeCaster Apr 26 '13 at 12:11
  • Yeah that would be problematic, but at that point you would just need to deep copy those objects via a `Select` maybe instead because calling `ToList`. That would work wouldn't it? – Mike Perrenoud Apr 26 '13 at 12:14
  • @MichaelPerrenoud I love editing comments, nevermind that. In the case of reference types you'd indeed probably want a deep copy of the list. – CodeCaster Apr 26 '13 at 12:19
  • 1
    Have a look at that link you gave me, I added a `DeepCopy` example. It's primitive, and wouldn't work for all types, but it would hit the 90% case. – Mike Perrenoud Apr 26 '13 at 12:34
3

Because you're adding the list object to the dictionary and then you're clearing the very same object that you added.

What you want, instead, is:

public void CalculateManhattanDistance(Dictionary<int, List<string>> MissingList, Dictionary<int, List<string>> OtherList)
    {
        Dictionary<int,Array> MissingListNeighbours = new Dictionary<int,Array>();
        Dictionary<int, List<int>> ManhattanDistanceList = new Dictionary<int,List<int>>();

        try
        {
            for (int i = 0; i < MissingList.Count(); i++)
            {
                List<int> tempList = new List<int>();
                for (int j = 0; j < OtherList.Count(); j++)
                {
                    int total=0;
                    for (int k = 0; k < MissingList[0].ToArray().Length; k++)
                    {
                        if (Convert.ToChar(MissingList[i][k].ToString()) == '?')
                            continue;
                        else
                            total += Math.Abs(Convert.ToInt32(MissingList[i][k].ToString()) - Convert.ToInt32(OtherList[j][k].ToString()));


                    }
                    tempList.Add(total);

                }
                ManhattanDistanceList.Add(i, tempList);

            }
        }
        catch (Exception ex)
        {
              ex.Message.ToString();
        }
    }

Make a habit of declaring variables at the scope in which they are needed and you won't run into this kind of problem often.

Theodoros Chatzigiannakis
  • 28,773
  • 8
  • 68
  • 104