locked
System.IndexOutOfRangeException was unhandled RRS feed

  • Question

  • Hello All,

    I'm trying to do some practice with C#, the main idea is to take a string turn it into a array and find the most common character in it,  but I'm having a issue with the for loops. when i start debugging i get the error System.IndexOutOfRangeException was unhandled and it points to the for loop. I've tried some things, but to no avail.

    Any Ideas ?

    using System;
    
    class SortString
    {
      public static void Main()
      {
        string str = "blah blah bllah bllllahh blah blahhh blah";
        SortStr(str);
        
      }
      /*****************************/
      /*****************************/
    
      public static char SortStr(string str)
      {
        int [] intArray = new int [255];
        char[] charArray = new char[str.Length];
        charArray = str.ToCharArray();
        int i;
        int max;
        int index;
        for(i = 0; charArray[i] != 0; i++)
        {
          ++intArray[charArray[i]];
        }
        max = intArray[0];
        index = 0;
        for (i = 0; charArray[i] != 0; i++)
        { 
          if( intArray[i] > max)
          {
            max = intArray[i];
            index = i;
          }
        }
        char result;
        result = Convert.ToChar(index);
        Console.WriteLine("this is the result " + result + (str.Length - 1));
        return result;
      }
    }
    
    Thanks in Advance

     

     

    Saturday, September 4, 2010 2:09 PM

Answers


  • There is no guarantee that a char will have a zero value.  Change your loops from what you have to:
     
        for (i = 0; i < charArray.Length; i++)
        {

    --
    Mike
    Saturday, September 4, 2010 2:22 PM
  • With just a glance ...

    You are dimensioning the charArray to the length of your string (which is 41 I believe), then your loop does this:

        for(i = 0; charArray[i] != 0; i++)
        {
          ++intArray[charArray[i]];
        }
    It continues until it finds an element that is not 0. There are no 0 elements in your string, so when i >=41 you get a subscript out of range error.

    You instead need this: 

          for (i = 0; i<str.Length; i++)
          {
            ++intArray[charArray[i]];
            Console.WriteLine(i);
          }
    

    Hope this helps.


    www.insteptech.com ; msmvps.com/blogs/deborahk
    We are volunteers and ask only that if we are able to help you, that you mark our reply as your answer. THANKS!
    Saturday, September 4, 2010 2:23 PM
  • > charArray[i] != 0

    Looks like you are expecting your string to end with a '\0', like in C/C++.  This is not the case in .NET.  Use the Length property:

    for (i = 0; i < charArray.Length; i++)
    

    There are other bugs in your code too, but since you are just practicing, I wouldn't want to deprive you from practicing your debugging skills.

     

    Saturday, September 4, 2010 2:28 PM

  • My recommendation was using: i < charArray.Length in the loops, not charArray[i] < charArray.Length;

    --
    Mike
    Saturday, September 4, 2010 3:21 PM
  • You are correct. Guess I am still asleep this morning. :-)

    Here is the code with the GroupBy:

          string str2 = "blahz blah bllah bllllahh blah blahhh blahz";
          var mostComonLetter = str2.GroupBy(n => n).
            OrderByDescending(g => g.Count()).
            Select(g => g.Key).FirstOrDefault();
          Console.WriteLine(("Result is: " + mostComonLetter)); 
    
    Notice I added a "z" to the string ensure I got the "l" as the answer because it was the most used, not the highest letter in the set.

    This code groups the like letters together. The it orders them (sorts) by their count. Then it takes the first one, which is the one used the most times.

    I have a blog post about this here:

    http://msmvps.com/blogs/deborahk/archive/2010/05/07/linq-mean-median-and-mode.aspx

    (Getting the value that occurs the most times is the "mode".)

    Hope this helps.


    www.insteptech.com ; msmvps.com/blogs/deborahk
    We are volunteers and ask only that if we are able to help you, that you mark our reply as your answer. THANKS!
    Saturday, September 4, 2010 3:29 PM

All replies


  • There is no guarantee that a char will have a zero value.  Change your loops from what you have to:
     
        for (i = 0; i < charArray.Length; i++)
        {

    --
    Mike
    Saturday, September 4, 2010 2:22 PM
  • With just a glance ...

    You are dimensioning the charArray to the length of your string (which is 41 I believe), then your loop does this:

        for(i = 0; charArray[i] != 0; i++)
        {
          ++intArray[charArray[i]];
        }
    It continues until it finds an element that is not 0. There are no 0 elements in your string, so when i >=41 you get a subscript out of range error.

    You instead need this: 

          for (i = 0; i<str.Length; i++)
          {
            ++intArray[charArray[i]];
            Console.WriteLine(i);
          }
    

    Hope this helps.


    www.insteptech.com ; msmvps.com/blogs/deborahk
    We are volunteers and ask only that if we are able to help you, that you mark our reply as your answer. THANKS!
    Saturday, September 4, 2010 2:23 PM
  • > charArray[i] != 0

    Looks like you are expecting your string to end with a '\0', like in C/C++.  This is not the case in .NET.  Use the Length property:

    for (i = 0; i < charArray.Length; i++)
    

    There are other bugs in your code too, but since you are just practicing, I wouldn't want to deprive you from practicing your debugging skills.

     

    Saturday, September 4, 2010 2:28 PM
  • Just a side note ... using Linq you can do all of that code in two lines:

          string str = "blah blah bllah bllllahh blah blahhh blah";
          var mostCommonCharacter = str.ToArray().Max();
    

    Hope this helps.


    www.insteptech.com ; msmvps.com/blogs/deborahk
    We are volunteers and ask only that if we are able to help you, that you mark our reply as your answer. THANKS!
    Saturday, September 4, 2010 2:30 PM
  • > mostCommonCharacter

    That gives the maximum character value, not the most frequent character.  Use group by -- or cleverly apply the Aggregate extension method for higher efficiency.

     

     

    Saturday, September 4, 2010 2:44 PM
  • Yea thanks for the help that does stop the error, however now it just goes over the for loops
    Saturday, September 4, 2010 3:12 PM
  • Can you post your revised code?
    www.insteptech.com ; msmvps.com/blogs/deborahk
    We are volunteers and ask only that if we are able to help you, that you mark our reply as your answer. THANKS!
    Saturday, September 4, 2010 3:14 PM
  • sure no problem

     

    using System;
    
    class SortString
    {
      public static void Main()
      {
        string str = "blah blah bllah bllllahh blah blahhh blah";
        SortStr(str);
        
      }
      /*****************************/
      /*****************************/
    
      public static char SortStr(string str)
      {
        int [] intArray = new int [255];
        char[] charArray = new char[str.Length];
        charArray = str.ToCharArray();
        int i = 0;
        int max;
        int index;
        for(i = 0; charArray[i] < charArray.Length; i++)
        {
          ++intArray[charArray[i]];
          Console.WriteLine(i);
        }
        max = intArray[0];
        index = 0;
        for (i = 0; charArray[i] < charArray.Length; i++)
        { 
          if( intArray[i] > max)
          {
            max = intArray[i];
            index = i;
          }
        }
        char result;
        result = Convert.ToChar(index);
        Console.WriteLine("this is the result " + result + (str.Length - 1));
        return result;
      }
    }
    

    Saturday, September 4, 2010 3:16 PM

  • My recommendation was using: i < charArray.Length in the loops, not charArray[i] < charArray.Length;

    --
    Mike
    Saturday, September 4, 2010 3:21 PM
  • indeed it is, i think I've been awake to long. I'm getting goofy now.

    Thanks for the help!!!!

    Saturday, September 4, 2010 3:28 PM
  • You are correct. Guess I am still asleep this morning. :-)

    Here is the code with the GroupBy:

          string str2 = "blahz blah bllah bllllahh blah blahhh blahz";
          var mostComonLetter = str2.GroupBy(n => n).
            OrderByDescending(g => g.Count()).
            Select(g => g.Key).FirstOrDefault();
          Console.WriteLine(("Result is: " + mostComonLetter)); 
    
    Notice I added a "z" to the string ensure I got the "l" as the answer because it was the most used, not the highest letter in the set.

    This code groups the like letters together. The it orders them (sorts) by their count. Then it takes the first one, which is the one used the most times.

    I have a blog post about this here:

    http://msmvps.com/blogs/deborahk/archive/2010/05/07/linq-mean-median-and-mode.aspx

    (Getting the value that occurs the most times is the "mode".)

    Hope this helps.


    www.insteptech.com ; msmvps.com/blogs/deborahk
    We are volunteers and ask only that if we are able to help you, that you mark our reply as your answer. THANKS!
    Saturday, September 4, 2010 3:29 PM
  • As I alluded to, here is one of the ways to do this with Aggregate, thus avoiding the sort.  This is admittedly rather "advanced".

    string str = "blahz blah bllah bllllahh blah blahhh blahz";
    var mostComonLetter = str.GroupBy(n => n).Aggregate(new { c = '\0', n = 0 }, (longest, g) => { int cnt = g.Count(); return cnt > longest.n ? new { c = g.Key, n = cnt } : longest; });
    Console.WriteLine(mostComonLetter); // Prints: { c = l, n = 11 }
    
    Saturday, September 4, 2010 4:31 PM
  • The second loop should go from 0 to intArray.Length.
    Saturday, September 4, 2010 11:41 PM