Sign up ×
Stack Overflow is a community of 4.7 million programmers, just like you, helping each other. Join them, it only takes a minute:

I'm pretty new to c# and I'm trying to make a function that checks if a certain number is in a list, and I want to run the function for every number between 1-10000. Currently it looks like this but i'm getting System.StackOverflowException so does anyone know how to do this correctly?

int number = 1; 
int maxnumber = 10000; 
void LoadFavorites()
{

    if (number <= maxnumber)
    {
        if (Properties.Settings.Default.FavoriteList.Contains("'"+number+"'"))
        {
            this.listBox1.Items.Add(number);
        }
    }

    // Increases number by 1 and reruns
    number = number + 1;
    LoadFavorites(); // problem is probably here
}
share|improve this question
1  
You're doing it recursively? Is that really what you want? Or do you just want to use a simple loop? – Jason Feb 7 '12 at 1:52

8 Answers 8

up vote 1 down vote accepted

You should be doing this in a loop, not using recursion. Each time you recurse it's going to add information to the stack and eventually you'll run out of stack space. In this case, you're recursing infinitely. (You're not stopping it from recursing if number > maxnumber).

void LoadFavorites()
{
    int number = 1; 
    int maxnumber = 10000; 
    while (number <= maxnumber)
    {
       if (Properties.Settings.Default.FavoriteList.Contains("'"+number+"'"))
       {
           this.listBox1.Items.Add(number);
       }
       number = number + 1;
    }
}

(edit: additional comment about "'"+number+"'" removed)

share|improve this answer
    
Thanks a lot, I'll use that instead. Although "'"+number+"'" is like that because I made the favorite saving system that way as well to avoid it for example thinking there's a favorite at number 23 when there's really one at 7236 or something – user1071461 Feb 7 '12 at 1:59
    
Ah ok, I'll strike out the rest of the info then. – rein Feb 7 '12 at 2:12

You are right. You have a recursive function there that does not have an appropriate stopping condition. Maybe you need a loop that goes from 1 to 100000 instead and there you call the function loadFavorites(). The stackoverflow is caused because you are calling loadFavorites() an infinite number of times, eventually you run out of stack space.

e.g.

    for(int i=number; i<maxNumber; i++)
    {
        if (Properties.Settings.Default.FavoriteList.Contains("'"+i+"'"))
            {
                this.listBox1.Items.Add(i);
            }

    }
share|improve this answer

There is no exit point, thus the stack overflow. You need to create a condition on the recursive call otherwise it will never be able to exit.

Example:

    void LoadFavorites()
    {

        if (number <= maxnumber)
        {
            if (Properties.Settings.Default.FavoriteList.Contains("'"+number+"'"))
            {
                this.listBox1.Items.Add(number);
            }
        }

        // Increases number by 1 and reruns
        number = number + 1;

        if(number <= maxnumber) // create a condition to call this
          LoadFavorites(); // problem is probably here
    }

OR a better approach

    void LoadFavorites()
    {

        if (number <= maxnumber)
        {
            if (Properties.Settings.Default.FavoriteList.Contains("'"+number+"'"))
            {
                this.listBox1.Items.Add(number++); // add number to list THEN increment number by one 
            }
            LoadFavorites(); 
        }

    }
share|improve this answer
    
I see... I'm such a retard, thanks – user1071461 Feb 7 '12 at 1:52

You want to move the part that increases the number to inside the if statement:

    int number = 1; 
    int maxnumber = 10000; 
    void LoadFavorites()
    {

        if (number <= maxnumber)
        {
            if (Properties.Settings.Default.FavoriteList.Contains("'"+number+"'"))
            {
                this.listBox1.Items.Add(number);
            }

            // Increases number by 1 and reruns
            number = number + 1;
            LoadFavorites(); // problem is probably here
        }
    }
share|improve this answer

You are calling LoadFavorites recursively without any rule to end recursion. Just, put it inside you if statement...But why don't you write it like this:

int number = 1; 
int maxnumber = 10000; 
void LoadFavorites()
{
    for(var i = number; i <= maxnumber; i++)
    {
        if (Properties.Settings.Default.FavoriteList.Contains("'"+number+"'"))
        {
            this.listBox1.Items.Add(number);
        }
    }
}
share|improve this answer

the problem is when number>100001, your code is still running(incrementing by 1 everytime and not exiting the loop). so change code to this:

 if (number <= maxnumber)
            {
                if (Properties.Settings.Default.FavoriteList.Contains("'"+number+"'"))
                {
                    this.listBox1.Items.Add(number);
                }
                number = number + 1;
                LoadFavorites();
            }
share|improve this answer
int number = 1; 
int maxnumber = 10000; 
void LoadFavorites()
{

    if (number <= maxnumber)
    {
        if (Properties.Settings.Default.FavoriteList.Contains("'"+number+"'"))
        {
            this.listBox1.Items.Add(number);
        }
    }

    // Increases number by 1 and reruns
    number = number + 1;
    LoadFavorites(); //**MOVE THESE:
}

Change to

int number = 1; 
int maxnumber = 10000; 
void LoadFavorites()
{

    if (number <= maxnumber)
    {
        if (Properties.Settings.Default.FavoriteList.Contains("'"+number+"'"))
        {
            this.listBox1.Items.Add(number);
            // Increases number by 1 and reruns
            number = number + 1;
            LoadFavorites(); // problem is probably here
        }
    }
}

You're calling a function from within a function, which, as you probably know, is recursion. With recursion, however, you have to have an exit condition, which causes the loop to exit.

The stack is the order of function calls in a thread. For example, in Java, you have main() which is your entry point. Calling functions from main() adds to the top of the stack. Once the stack gets to a certain size, your computer can no longer buffer more function calls. It simply drops the main function from the stack, and doesn't know where to return to, so it throws an error. (It throws the error once 'main' is dropped, not once it tries to return).

The reason for the change is that you want to continue only if number is < maxnumber. This sets up a condition for the recursion, after which, the function will exit.

share|improve this answer

The issue is that whenever you call a function in c++, it takes up some memory (it needs space to store its variables, among other things). This memory is called a stack frame. The memory is automatically returned when the function returns, but if you have a recursive function, none of the stack frames can be released until the final function call happens. In this case, LoadFavorites tries to call itself infinitely, and your computer does not contain an infinite amount of memory.

In C++, the best answer generally is to convert this into a loop - in this case, you want a for loop.

void LoadFavorites() {
    int maxnumber = 10000;
    int number;
    for (number = 1; number <= maxnumber; number++) {
        if (Properties.Settings.Default.FavoriteList.Contains("'"+number+"'")) {
            this.listBox1.Items.Add(number);
        }
    }
}

Alternately, you could move the call to LoadFavorites into the if statement so that you don't try to call LoadFavorites an infinite number of times. This could work, but depending on the number times you call LoadFavorites, you could still run into the same error. The error can be avoided with the right compiler optimizations, but it generally is safer and clearer to simply use a loop here.

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.