What condition can I use to terminate my recursion?

Question

I have a problem with a recursion in my program.

I want to randomly find a word in a list of words, but I want the word to have fewer than six letters if the difficulty level is on "easy", and more than six for level "hard".

I know I'm supposed to have a break point, but since I don't know how many times it might loop before the user finds a good word, I don't know what to do.

How can I terminate my recursion?

 private void trouverMot()
    {

        var random = new Random();
        int index = random.Next(0,maList.Count);
        mot = (maList[index].Trim());

        if(niveau == "Facile")
        {
            if(mot.Length > 6 || lstUse.Contains(mot))
            {
                trouverMot();
            }

        }else
        {
            if(mot.Length < 6 || lstUse.Contains(mot))
            {
                trouverMot();
            }
        }

        lstUse.Add(mot);
        affichage();

    }

I don't understand what your question is, this code seems to be working correctly — Liora Haydont, Dec 22 '17 at 21:50
@Hug If this code goes into an infinite lop after a couple of runs, you may need to increase the number of items available for picking. — Sergey Kalinichenko, Dec 22 '17 at 23:27
Why are you trying to shoe-horn recursion into this code? You can do this much simpler with a simple loop. — Blorgbeard, Dec 23 '17 at 00:51
Probably unrelated, but you shouldn't create a new `Random` every time. You should create one, either in a class-level field, or in the code that calls this method, and pass it as an argument. If your code executes fast enough the way you have it, you may end up with bad randomization. — Bradley Uffner, Dec 23 '17 at 01:35
Ok thanks for the help guys, that means my problem might be somewhere else. — Hug, Dec 23 '17 at 02:15
Nice seems like get the create new Random out seems to have fix my problem :) — Hug, Dec 23 '17 at 02:27

Sergey Kalinichenko · Answer 1 · 2017-12-23T01:00:27.883

0

You would be better off splitting hard words from easy ones. Rather than keeping a single maList, populate two separate ones, facileList and difficileList with words depending on their lengths. Now you can limit the number of recursive invocations at some random level, say, after 10 retries. Then you'd be able to rewrite your method as follows:

private static Random random = new Random();
private void trouverMot(int retry) {
    var list = niveau == "Facile" ? facileList : difficileList;
    int index = random.Next(0, list.Count);
    mot = (list[index].Trim());
    if(!lstUse.Contains(mot) || retry == 0) {
        lstUse.Add(mot);
        affichage();
        return;
    }
    trouverMot(retry-1);
}

edited Dec 23 '17 at 01:00

answered Dec 22 '17 at 21:58

Sergey Kalinichenko

714,442
84
1,110
1,523

Good idea but that means you always need a limit of retry in a recursion like that? – Hug Dec 22 '17 at 22:26
Is there a way i can tell him to stop when he find a good word? – Hug Dec 22 '17 at 22:27
@Hug It would already stop after finding a non-repeating word (see `return`). It would continue only if it does not find a non-repeating word, until it exhausts the limit of retries. The limit is there to ensure that your code is guaranteed to stop. It wouldn't stop if all available words have been used in `lstUse`. – Sergey Kalinichenko Dec 22 '17 at 22:35

score 0 · Answer 2 · answered Dec 23 '17 at 01:12

On the off-chance that you are not obliged by your instructor to use recursion for this, here is a working non-recursive version of your code:

private static Random random = new Random();

private void trouverMot()
{
    while (true) 
    {
        int index = random.Next(0,maList.Count);
        mot = (maList[index].Trim());

        if (niveau == "Facile")
        {
            if (mot.Length > 6 || lstUse.Contains(mot))
            {
                continue;
            }
        }
        else
        {
            if (mot.Length < 6 || lstUse.Contains(mot))
            {
                continue;
            }
        }
    }

    lstUse.Add(mot);
    affichage();
}

Using recursion in order to "restart" a function is not a common idiom, and although it seems simpler, it can cause issues (e.g. StackOverflowException).

Note: I've changed only the recursion (and moved the Random out - see this answer) - other improvements suggested by dasblinkenlight@ also apply to this.

What condition can I use to terminate my recursion?

2 Answers2