Longest Common Subsequent output wrong result

Question

I've been trying to solve the LCS problem using recursion like below:

#include <iostream>
#include <string>

using namespace std;

string max(string x, string y) {
    return x.size() <= y.size() ? y : x;
}

string find(string x, string y) {
    // Find longest Sub Sequence
    int firstSize = x.size();
    int secondSize = y.size();

    if (firstSize == 0 || secondSize == 0) {
        return "";
    }

    if (x[firstSize - 1] == y[secondSize - 1]) {

        char temp = x[firstSize - 1];

        return find(x.erase(firstSize-1), y.erase(secondSize-1)) + temp;
    }

    return max(find(x, y.erase(secondSize - 1)), find(x.erase(firstSize - 1), y));
}

int main()
{
    string x = "ABCBDAB";
    string y = "BDCABA";

    string result = find(x, y);

    cout << "result = "<<result<< endl;

    return 0;
}

The algorithm looks exactly, but the output is ABA subsequent that is wrong result, so something go wrong. The result is expected be a subsequent has length of 4 and I don't know exactly where I went wrong with the code. Why is it that ?

Have you tried debugging yet? Try to perform the algorithm with pan and paper and compare it with what you expect and what your implementation does. — MrSmith42, Mar 24 '20 at 08:34
Yes, I wrote down on a piece of paper, but had too many sub problems and the result was also difference from compiler. Maybe I will try with smaller input. Thank for your advice ! — Hieu Na, Mar 24 '20 at 08:47

score 1 · Answer 1 · answered Mar 24 '20 at 09:25

1

I'd try without erasing the char and instead using arrays - max(find(x, y[yIndex - 1]), find(x[xIndex-1], y)).

answered Mar 24 '20 at 09:25

Rauf Aghayev

300
1
12

score 1 · Accepted Answer · 2020-03-25T22:11:34.870

I cleaned up your code a bit and let us go trough what I noticed:

#include <iostream>
#include <string>

using namespace std;

I personally would've avoided question mark based return values in code, because they are not that readable, but this is just a suggestion.

string max(string x, string y)
//  Return biggest string
{
    if (x.size() <= y.size())
        return y;
    else
        return x;
}

string find(string x, string y)
//  Find longest Sub Sequence
{
    int xSize = x.size();
    int ySize = y.size();

Let's say the two string were both "ABC". Now the second if statement will return a find on AB call, which will return a find on A call, which will return "" as the longest substring of A and A, resulting in "" as the longest substring for ABC and ABC. Guess you weren't after this.

    if (xSize == 0 || ySize == 0)
        return "";

That's why I adapted the second if statement to what you meant, probably. Here you were erasing the last character, then adding the second last character back; I'm not sure if that was supposed to be, but since y[last] equals x[last], appending y after removing it's last with the last character of x is y itself.

    else if (x[xSize - 1] == y[ySize - 1])
        return find(x.erase(xSize - 1), y);

Then there was this variable only being used once: char temp = x[xSize - 1]. You might want to write it inline, so you don't have to make a variable and comment if necessary; but that's just code-style preferences.

    else
        return max(find(x, y.erase(ySize - 1)), find(x.erase(xSize - 1), y));
}

int main()
{
    string x = "ABCBDAB";
    string y = "BDCABA";

    cout << "result = " << find(x, y) << endl;

    return 0;
}

Try to structure your code a bit more, and now refine the algorithm, because there are some algorithm mistakes in it. Goodluck!

I really appreciate your help in resolving the problem. Finally, I solved it by replacing erasing the char by using index m, n as parameters that represent size of the strings. — Hieu Na, Mar 24 '20 at 14:41

Longest Common Subsequent output wrong result

2 Answers2