Is this a bad use of a recursive function?

[Jsplinter]

Below is a simplified version of the function.

int x;
void myAdd(const double& p)
{
    if (x > 100)
    {
        x = 1;
        myAdd(p);
    }
    ++x;
    ...
}

The advantage of this recursion is that I don't have to write a second function ie myAdd2.
Are there any pitfalls of recursive functions that I should be aware of?

[Fbody]

Your post is somewhat incomplete, making it difficult to make sense of. In and of itself, this snippet is a very bad piece of code:

1. You're trying to base a comparison on an uninitialized variable (variable 'x').
2. You're trying to increment it when you don't even know if the value it contains is a valid value to begin with.
3. You have a function parameter that never gets used.

I know that 'x' is a global variable, but what I'm seeing here isn't using it properly.

Also, what is the purpose of 'p' if you're not going to do anything with it?

I think you need to explain your situation and reasonings better, "I don't have to write a second function" is a marginal advantage at best.

So far, this sounds more like favoring laziness, complexity and confusion over effort, simplicity, and readability.

It's really not possible to say if this is a good use of recursion without a better description of your intent.

[Jsplinter]

Right on. I apologize for the poorly written post. I've just never felt comfortable with recursion, and I was trying to distill my question down to its most basic elements. The ellipses at the bottom of the function is where p gets "used."

int x is indeed initialized to 0. For my part the advantage of not writing a second function that is nearly identical to the original is that it improves readability and maintainability of the code.

As I haven't clearly stated my question about recursion, I in fact don't know exactly why it makes me nervous, I'm just going to going to step through the program as it is written and see what happens. Thank you.

[Fbody]

Based on your responses, I have concerns about whether your recursion will ever start. Also, based simply on the posted code, if it does start, it will probably not terminate because it's based on the value of x, which never changes once the recursion starts. Barring other manipulations else where in your code, once x hits 101, the recursion will start and never stop until it either crashes your program or, even worse, your computer by running it out of memory.

[Jsplinter]

I don't understand why it would start and never stop once x hits 101.

int x = 0;

void myAdd(const double& p)
 {
    if (x > 100)
    {
        x = 1;
        myAdd(p);
    }
    ++x;
    ... (do something to p)
}

When x hits 101, it gets reset to 1. myAdd() should then only be called once by recursion, as far as I can tell.

[Fbody]

Oops, I forgot about the assignment statement.

This function still doesn't make sense to me though. What are you trying to do that makes you think recursion is needed over a simple loop?

[mrnutty]

Dude just use if( x % 100 == 0){ x= 1; } else ++x;

[mike_2000_17]

Maybe I'm crazy, but isn't your code just equivalent to something like this:

int x = 0;

void myAdd(const double& p)
 {
    x = x % 101;
    ++x;
    ... (do something to p)
}

[Jsplinter]

Thank you. You have completely answered the question as originally stated. I apologize for leaving out information that might have cleared things up.

In case you are still curious, I'll try to explain myself better now. lrPoint is a struct which contains an int and a double. lrPointDeq is a Deque which holds the last period of lrPoints. The purpose of this function is to calculate a linear regression on a sequence of numbers. When x gets really large, ie a lot bigger than 100, it resets x to 0, and then runs through the deque with a for loop to recalculate, matching the previous period of p values with new x values.

int x = 0;
 
void myAdd(const double& p)
 {
    if (x > 100)
    {
        x = 0;
        for(deque<lrPoint>::iterator it = LRPointsDeq.begin(); it != LRPointsDeq.end(); ++it)
			{
				myAdd(it->yVal);  // recursion here or add 35 lines of code??
			}
    }
    ++x;
    LRPointsDeq.push_back((lrPoint(x, p)));
    ... (more code)
    ...
    ...
}

Yes, I could do everything within the loop, BUT I think the recursion makes more sense. Maybe I am wrong. It would just mean copying about 35 lines of code into the for loop. hmmm, the more I write the sillier I feel. So, the question boils down to this. If recursion makes a function fit on a single screen instead of two, is it a good idea?

[Jsplinter]

Maybe I'm crazy, but isn't your code just equivalent to something like this:

int x = 0;

void myAdd(const double& p)
 {
    x = x % 101;
    ++x;
    ... (do something to p)
}

yes! you are correct. I can see that now.

[mike_2000_17]

Having more functions is not more complicated than having less. As long as the code does not get copy-pasted. Recursion is not the answer in this case. I would probably do it like this:

int x = 0;
 
void myAdd_no_check(const double& p) {
    ++x;
    LRPointsDeq.push_back((lrPoint(x, p)));
    ... (more code)
    ...
    ...
};

void myAdd(const double& p)
 {
    if (x > 100)
    {
        x = 0;
        for(deque<lrPoint>::iterator it = LRPointsDeq.begin(); it != LRPointsDeq.end(); ++it)
        {
            myAdd_no_check(it->yVal);
        }
    }
    myAdd_no_check(p);
}

If this function is a member function of a class, I would probably make the "no_check" version private. Otherwise, you can also hide it in a "detail" namespace such that users don't call it directly.

The reason why recursion is not useful here is that the value of x is reset, so when calling the function again, you have a useless check and no real possibility of a recursion of more than one layer. Of course, if all those lines of code end up modifying x to the point that it could reach 100, then the recursive call might be necessary. However, I have to point out the the behaviour of this recursion based on the value of a global variable is really weird (almost undefined). Try to change the design such that a global variable like 'x' is not needed.