current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite

I have a program that calculates some values and saves them to an array. Before save a value, program checks if there is already an identical value in array:

 string someData = GetData();
 bool newItem = true;
 int arrayDataLength = arrayData.Length;
 for (int x = 0; x < arrayDataLength; x++)
 {
     if (arrayData[x] == someData)
     {
         newItem = false;
         arrayDataCount[x]++;
         break;
     }
  }
  if (newItem == true)
  {
       Array.Resize(ref arrayData, (arrayDataLength + 1));
       Array.Resize(ref arrayDataCount, (arrayDataLength + 1));
       arrayData[arrayDataLength] = someData;
       arrayDataCount[arrayDataLength]++;
  }

Sometimes arrays with data become big, and program works a bit slow. How can I optimize perfomance of this action?

share|improve this question
1  
I would strongly suggest waiting at least a day before accepting an answer (even if it is very helpful), as you are likely to get more feedback. –  codesparkle Jan 16 '13 at 14:00
    
Yes, I see it now. –  ahawkthomas Jan 17 '13 at 5:41

3 Answers 3

up vote 4 down vote accepted

If you don't need to count the number of occurences, use a Set:

private readonly ISet<string> data = new HashSet<string>();

public void AddNewData()
{
    string someData = GetData();
    data.Add(someData);
}

If you need to count the occurences, you should use a Dictionary:

private readonly IDictionary<string, int> data = new Dictionary<string, int>();

public void AddNewData()
{
    string someData = GetData();
    if (data.ContainsKey(someData))
    {
        data[someData]++;
    }
    else
    {
        data[someData] = 1;
    }
}

Alternatively, you could just use a List and calculate the counts when you need them, using the LINQ GroupBy extension method:

private readonly ICollection<string> data = new List<string>();

public void AddNewData()
{
    string someData = GetData();
    data.Add(someData); // data might contain duplicates
}

public void PrintData() // use LINQ to count number of occurences
{
    foreach (var entry in data.GroupBy(x => x))
    {
        Console.WriteLine("{0} {1}", entry.Key, entry.Count());
    }
}
share|improve this answer
up vote 2 down vote

1. Resizing. 

Array.Resize(ref arrayData, (arrayData.Length+1);

It's not a good way to resize an array. You copy the whole array every time you need add just one value. You might want to consider the approach which is used in .NET collections. If you need to add something to an array and there is not enough space - double it's size. However, in this case you need to distinguish between capacity (allocated memory) and length (the real number of stored elements)

int newRealLength = realLength + 1;
if (arrayData.Length < newRealLength) {
   Array.Resize(ref arrayData, arrayData.Length*2);
}
realLength = newRealLength;

Then, in the end, just drop everything above realLegth in your array by resizing it again.

Array.Resize(ref arrayData, realLength);

Otherwise, you might want to consider List - it uses an array underneath and employs the strategy I've described above - you don't have to write anything on your own in this case.

2. Search

Search in the whole array is also a problem. If order doesn't matter and you can deal with duplicates later. You can do the trick. 

Dictionary<string,int> d = new Dictionary<string,int>();
if (false == d.ContainsKey(someData)) { 
  d.Add(someData, 0);
} else {
  int count = d[someData];
  d[someData] = count + 1;      
}
//later you can convert keys and values to the 2 independent arrays
//1st - added items
//2nd - number of duplicates 
String[] arrayData = d.Keys.ToArray();
int[] arrayDataCount = d.Values.ToArray();

Please notice that this approach also covers the problem of resizing but doesn't preserve processing order.

share|improve this answer
    
Thanks very much for your reply! The way of using Dictionary is good for my task because I really have key-value pairs. –  ahawkthomas Jan 16 '13 at 11:38
2  
Instead of false == something, write just !something. Also Yoda contions are usually not used in C#, because they are not needed. –  svick Jan 16 '13 at 14:04
    
In .NET, Dictionary doesn't have a Get method. And Contains won't compile because it expects a KeyValuePair. See my answer for the correct version. –  codesparkle Jan 16 '13 at 15:12
    
@codesparkle, thanks for the note - fixed. –  Andrey Taptunov Jan 16 '13 at 15:23
    
Not quite. Add throws an ArgumentException if the key already exists. –  codesparkle Jan 16 '13 at 18:25
up vote 1 down vote

You should use List<String> to replace your array to overcome resize problem.

It will automatically resize the internal array to double the capacity when existing capacity becomes full.

Your code will become

   List<String> list = new List<String>(); // add values to it.
   if (!list.Contains(someData))
   {
        list.Add(someData);
   }

To efficiently search, You should use Dictionary<TKey, TValue>

   Dictonary<String, String> dict = new Dictionary<String, String>(); // add values to it.
   if (!dict.ContainsKey(someData)) // if you have key, use dict.ContainsKey(key)
   {
        dict.Add(someData, someData); // If you have key, use dict.Add(key,someData);
   }
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.