-1

I want a function that returns all possible combinations of an alphabet X, the combinations have to be n long. I understand this had been done a million times before but I try to understand recursion.

Two things I don't understand:

One. The function inserts every last iteration of a series twice, so if alphabet is 'AB' and length is 2 I would expect:

AA
AB
BA
BB

but I get:

AA
AB
AB
BA
BB
BB

and 2. How can the function need the return within the outer loop. If I just remove the outer loop and leave return then I get segmentation fault.

In the function I keep the vector with me, I could also put it in global scope but the I have the same effect. I build the possible combination S on the fly and keep len en depth:

#include<iostream>
#include<vector>
#include<cstring>
using namespace std;

void combirec(vector<string> &A, char S[20], char *alphabet, int len, int depth)
{
    for (int i = 0; i < len; i++) {
        for (int c = 0; c < strlen(alphabet); c++) {
            S[depth] = alphabet[c];
            combirec(A, S, alphabet, len-1, depth+1);
            A.push_back(S);
        }
        return;
    }
}

int main()
{
    vector<string> A;
    char S[20];
    char alphabet[6] = "AB";
    int len = 2;
    S[len] = '\0';
    combirec(A, S, alphabet, len, 0);
    for (int i = 0 ; i < A.size(); i++)
        cout << A[i] << endl;
}
Jarod42
  • 173,454
  • 13
  • 146
  • 250
Niels
  • 466
  • 3
  • 19
  • Why do you have both loops and recursion? You should have one or the other as they both do the same thing. – stark Mar 21 '19 at 12:59
  • This may be what I cannot oversee :S – Niels Mar 21 '19 at 13:00
  • 3
    It sounds like you may need to learn how to use a debugger to step through your code. With a good debugger, you can execute your program line by line and see where it is deviating from what you expect. This is an essential tool if you are going to do any programming. Further reading: [How to debug small programs](http://ericlippert.com/2014/03/05/how-to-debug-small-programs/) and [Debugging Guide](http://idownvotedbecau.se/nodebugging/) – NathanOliver Mar 21 '19 at 13:01
  • If I remove the inner loop I only get AB and AB once – Niels Mar 21 '19 at 13:03
  • 1
    You should *probably* have a look at [`std::next_permutation`](https://en.cppreference.com/w/cpp/algorithm/next_permutation) – Default Mar 21 '19 at 13:09
  • The thing is I want to do this myself. I know I can do it in 1 or two lines using all kinds of libraries – Niels Mar 21 '19 at 13:12
  • 1
    The outer loop with a return at the end is equivalent to `if (len > 0) { ... inner loop ... }`. Without it, you recurse forever (that is, until you run out of stack space). – molbdnilo Mar 21 '19 at 13:13
  • That is true, but with if (len>0) I just get the same result – Niels Mar 21 '19 at 13:14
  • 1
    @Niels Of course you get the same result; they are equivalent. But you shouldn't use a one-iteration loop in a language that has conditionals. – molbdnilo Mar 21 '19 at 13:15
  • The suggestion led to a working solution though. I will post it below. – Niels Mar 21 '19 at 13:17

2 Answers2

1

Your issue is that have both loops and recursion:

void combirec(vector<string> &A, char S[20], char *alphabet, int len, int depth)
{
    for (int i = 0; i < len; i++) {//<--LOOP HERE
        for (int c = 0; c < strlen(alphabet); c++) //{<--LOOP HERE
            S[depth] = alphabet[c];
            combirec(A, S, alphabet, len-1, depth+1);//<--RECURSIVE HERE
            A.push_back(S);
        }
        return;
    }
}

If you wan't recursion you probably need at most one loop.

See this answer for a similiar problem:

Creating all possible k combinations of n items in C++

darune
  • 9,436
  • 1
  • 16
  • 47
1

Some useful suggestions led to a solution, although why this works remains vague to me. I would never have come up with this myself.

void combirec(vector<string> &A, char S[20], char *alphabet, int len, int depth)
{
    if (len > 0)
        for (int c = 0; c < strlen(alphabet); c++) {
            S[depth] = alphabet[c];
            combirec(A, S, alphabet, len-1, depth+1);
        }
    else
        A.push_back(S);

}
Niels
  • 466
  • 3
  • 19