Szukanie najdłuższego wspólnego podciągu, poprawne alokacje pamięci i wydajność

0

Witam,
próbuje wykonać zadanie http://pl.spoj.com/problems/LENLCS/ ale coś mi nie chce wyjść, testy wykonuje poprawnie, ale na spoj daje mi SIGABRT. Korzystam z algorytmu z tej strony https://www.ics.uci.edu/~eppstein/161/960229.html który to jest ostatni (Space-efficient LCS - Reduced Space complexity) ponieważ chciałem mieć jak największą wydajność, a mój poprzedni przekraczał limit czasu. Moje pytanie brzmi, jak ten program poprawić i udoskonalić bo już sam nie wiem. Aktualnie valgrind nie pokazuje żadnych errorów i memory leak'ów.
Algorytm z tej strony wygląda następująco, w oryginalnej formie:

Space-efficient LCS:

    int LCS(char * A, char * B)
    {
	allocate storage for one-dimensional arrays X and Y
	for (i = m; i >= 0; i--)
	{
	    for (j = n; j >= 0; j--)
	    {
		if (A[i] == '\0' || B[j] == '\0') X[j] = 0;
		else if (A[i] == B[j]) X[j] = 1 + Y[j+1];
		else X[j] = max(Y[j], X[j+1]);
	    }
	    Y = X;
	}
	return X[0];
    }

Wykonałem małe modyfikacje, ponieważ używam operatora new i zostałem zmuszony użycia zmiennej bool'owskiej aby usunąć wskaźnik Y w pierwszym obiegu pętli unikając memory leak'a.
W obecnej formie, mój program wygląda następująco, prosiłbym o jakieś wskazówki co do niego.

 
#include <iostream>
#include <string>
#include <cstdio>
#include <cstring> // strlen
#include <algorithm> // max

using namespace std;

int LCS(const char * A, const char * B, int m, int n) {
	int i, j;
	bool toDestroy = false;  // do ubicia Y
	
        // alokacje pamieci
	int * X = 0;
	int * Y = 0;
	if(X == 0 && Y == 0) {
		X = new int[m+1];
		Y = new int[n+1];
	}
	
        // algorytm
	for (i = m; i >= 0; i--) {
	    for (j = n; j >= 0; j--) {
			if (A[i] == '\0' || B[j] == '\0') X[j] = 0;
			else if (A[i] == B[j]) X[j] = 1 + Y[j+1];
			else X[j] = max(Y[j], X[j+1]);
	    }
	    if(!toDestroy) { // do ubicia tego na co wskazuje Y w pierwszym obiegu petli, dzieki temu nie mam memory leak
			delete [] Y;
			toDestroy = true;
		}
	    Y = X;
	}
	int result = X[0];
	
	if(X != 0 && Y != 0) {  // sprzatanie
		delete [] X;
	}
	X = 0;
	Y = 0;
	
	return result; // zwroc wynik
}

int main() {
	std::ios_base::sync_with_stdio(0);
	int t, n;
	int a, b;
	string s1, s2;
	cin >> t;
	
	for(int i = 0; i < t; ++i) {
		cin >> a >> ws; // dlugosc s1
		getline(cin, s1);  // pierwszy wyraz
		cin >> b >> ws; // dlugosc s2
		getline(cin, s2);  // drugi wyraz
		
		cout << LCS(s1.c_str(), s2.c_str(), a, b) << endl;  // uruchomienie algorytmu
	}
	
	return 0;
}

Z góry dzięki za pomoc :)

0
  1. WTF
    int * X = 0;
    int * Y = 0;
    if(X == 0 && Y == 0) {
        X = new int[m+1];
        Y = new int[n+1];
    }

Co to niby jest? Obfuskacja jaka?

int* X = new int[m+1];
int* Y = new int[n+1];
        for (j = n; j >= 0; j--) {
            if (A[i] == '\0' || B[j] == '\0') X[j] = 0;
            else if (A[i] == B[j]) X[j] = 1 + Y[j+1];
            else X[j] = max(Y[j], X[j+1]);
        }

Y[j+1]? Poważnie? A jak j=n to co? To Y[n+1]? Poza tablicą? Brawo. Zapewne tutaj leży twój segfault.
3. Zamiast jakiegoś chorego "toDestroy" wystarczyło zrobić jeszcze jeden wskaźnik, nic więcej. Co to za problem zapamiętać sobie gdzieś na boku tego Y drugi raz? ;]
4. Znów WTF

    if(X != 0 && Y != 0) {  // sprzatanie
        delete [] X;
    }
    X = 0;
    Y = 0;

Zupełnie zbędne i bez sensu. Zrób normalne delete i już.

1

Do powyższego można jeszcze dodać tylko indeksacje:
Ponieważ rozmiar X jest m oraz iterujesz poprzez i od m do 0 to możesz indeksować X tylko poprzez i.
Więc jeżeli w kodzie masz X[j] to masz ewidentny błąd.

No i przy okazji ten błąd masz na własne życzenie. Zmień nazewnictwo:

X -> Xstr
m -> Xsize
i -> x
Y -> Ystr
n -> Ysize
j -> y

i natychmiast błąd opisany wyżej staje się niemożliwy do popełnienia, ponieważ jak zobaczysz Ystr[x] lub Xstr[y] to w głowie natychmiast coś zaświta.

1 użytkowników online, w tym zalogowanych: 0, gości: 1