Transponowanie macierzy

Cześć, piszę sobie klasę do obsługi działań na macierzach:

class Macierz
{
public:
	Macierz(int a, int b);
	Macierz(int a, int b, int c);
	~Macierz();
	void wypisz(void) const;
	void transponuj(void);

private:
	int **tab;
	int x;
	int y;
}; 

I chcę napisać metodę 'transponuj' która zrobi transponowanie macierzy (wiersz to kolumna, a kolumn to wiersz).

void Macierz::transponuj(void)
{
	int **tab_trans = new int *[y];
	for (int k = 0; k < y; ++k)
	{
		tab[k] = new int[x];
	}
	for (int i = 0; i < y; i++)
	{
		for (int j = 0; j < x; j++)
			tab_trans[i][j] = tab[j][i]; //tu wywala!
	}
	tab = tab_trans;
	swap(x, y);
	//tu jeszcze zwolnienie pamięci dla tab_trans
} 

W komentarzu zaznaczyłem miejsce w którym program sie wysypuje, mógłby ktoś spojrzeć i dać wskazówkę co jest nie tak?

    int **tab_trans = new int *[x];
    for (int k = 0; k < x; ++k)
    {
        tab[k] = new int[y];
    }

Oraz zapomniałeś zwolnić tab wraz z wierszami.

No tak, ale wydaje mi się że problem nie leży w samym utworzeniu tab_trans (tablica ta ma mieć rozmiar odwrotny do tab), tylko w przypisaniu do tab_trans, odpowiedniej wartości tab.

Tak jak pisze @_13th_Dragon masz memleak, bo nie zwalniasz tablicy (http://en.wikipedia.org/wiki/Memory_leak).
To nie bezpośrednio powód tego że Ci nie działa, to kolejny błąd. Powinieneś zwalniać tab zanim zmienisz jego wartość.

Jak o zwalnianiu mowa:

//tu jeszcze zwolnienie pamięci dla tab_trans

Jeśli fatycznie zwalniasz pamięć dla tab_trans, to zwalniasz całą tablicę (w tym momencie tab_trans pokazuje na to samo na co tab) - nic dziwnego że nie działa.

Jeszcze możesz się przyjrzeć czy x i y są poprawnie inicjalizowane.

Mój pomysł przedstawiał się następująco:

• mam tablicę którą chcę transponować, powiedzmy tab[x][y]
• rezerwuję sobie miejsce na nową tablicę, tylko z odwróconymi rozmiarami tab_trans [y][x]
• przepisuje wartości tab do tab_trans
• zwalniam pamięć dla starej tab
• niech tab pokazuje na to samo co tab_trans (tab = tab_trans)
• zwalniam pamięć dla tab_trans

Nie ma tutaj chyba mowy o wyciekach pamięci?

• zwalniam pamięć dla starej tab

W kodzie który podałeś tego nie ma, dlatego zauważamy. Tak, z tym nie byłoby wycieku pamięci.

• zwalniam pamięć dla tab_trans

Ale w ten sposób zwalniasz pamięć dla tego co jest aktualnie.

Prościej będzie opisać strzałkami. Przy wskaźnikach zawsze sprawdzają się strzałki ;)

początkowy stan:

tab -----------> początkowa tablica

alokowanie pamięci:

tab -----------> początkowa tablica
tab_trans  ----> nowa tablica

po pętli for robiąca transponowanie:

tab -----------> początkowa tablica
tab_trans  ----> tablica po transponowaniu (wynik)

po zwolnieniu pamięci dla tab:

tab -----------> [zwolniona pamięć, śmieci]
tab_trans  ----> tablica po transponowaniu (wynik)

tab = tab_trans:

tab ---------\
              -----> tablica po transponowaniu (wynik)
tab_trans  --/

I to wszystko czego potrzeba - tab wskazuje już na wynik końcowy, a tab_trans (tzn. sam wskaźnik, cztery/osiem bajtów) zniknie za chwilę bo jest zmienną lokalną.

Ale jeśli teraz zwolnisz tab_trans (czyli pamięć na którą wskazuje), to skończysz z (katastrofa):

tab ---------\
              -----> [zwolniona pamięć, śmieci]
tab_trans  --/

Okej, jeżeli chodzi o alokację pamięci wszystko już rozumiem, dzięki.

Jednak problemem jest teraz przepisanie wartości ze starej tablicy do nowej, gdyż np. linijka tab_trans[i][j] = tab[j][i] nie działa, czyli trzeba tu wymyślić chyba jakiś sprytniejszy sposób, niż ten znany ze statycznych tablic.

Mógłbyś dać cały kod macierzy? Bo w tym co podałeś nie ma (albo nie widzę na pierwszy rzut oka) błędu, może coś wcześniej idzie źle.

 class Macierz
{
public:
	Macierz(int a, int b);
	Macierz(int a, int b, int c);
	~Macierz();
	void wypisz() const;
	void transponuj();
	//void wyznacznik();

private:
	int **tab;
	int x;
	int y;
};

#include "Macierz.h"
#include <iostream>

using namespace std;

Macierz::Macierz(int a, int b)
{
	x = a;
	y = b;
	tab = new int *[a];
	for (int i = 0; i < a; ++i)
	{
		tab[i] = new int[b];
	}
}

Macierz::Macierz(int a, int b, int c)
{
	x = a;
	y = b;
	tab = new int *[a];
	for (int i = 0; i < a; ++i)
	{
		tab[i] = new int[b];
	}
	//wypełnienie tablicy argumentem c
	for (int i = 0; i < a; i++)
	{
		for (int j = 0; j < b; j++)
			tab[i][j] = c;
	}
}


Macierz::~Macierz()
{
	for (int i = 0; i < x; ++i)
	{
		delete tab[i];
	}
	delete tab;
}

void Macierz::wypisz(void) const
{
	for (int i = 0; i < x; i++)
	{
		for (int j = 0; j < y; j++)
		{
			cout << tab[i][j] << " ";
		}
		cout << endl;
	}
}

void Macierz::transponuj()
{
	int **tab_trans = new int *[y];
	for (int k = 0; k < x; ++k)
	{
		tab[k] = new int[x];
	}
	for (int i = 0; i < y; i++)
	{
		for (int j = 0; j < x; j++)
		{
			tab_trans[i][j] = tab[j][i]; //tu wywala!
		}
	}
	tab = tab_trans;
	swap(x, y);
} 

Jeszcze nie wcieliłem w życie (w kod), w metodzie transponuj zwalniania pamięci, o którym rozmawialiśmy, ale to chyba nie jest powód tego że program się wysypuje?

    int **tab_trans = new int *[y];
    for (int k = 0; k < x; ++k)

Hint: warunek pętli.

PS nazywaj sensowniej parametry;
PPS //wypełnienie tablicy argumentem c - c jest parametrem, nie argumentem;
PPPS raczej rozmiar macierzy nie może być ujemny, więc int jest nie na miejscu. unsigned int/size_t/(...) byłoby lepszym wyborem.

Tak tak, powinno być:

 
int **tab_trans = new int *[y];
	for (int k = 0; k < y; ++k)

Ale mimo to dalej wysypuje się w linijce przepisanie wartości: "Instrukcja spod <adres> odwołuje się do pamięci pod adresem <adres>. Pamięć nie może być written".

Zauważ jeszcze, do czego przypisujesz:
tab[k] = new int[x];

Faktycznie, nie zauważyłem tego (i nikt inny wcześniej) (chciaż problem jest trochę niżej):

int **tab_trans = new int *[y];
for (int k = 0; k < y; ++k)
{
    tab[k] = new int[x]; // chcesz stworzyć tablicę 2d z /tab_trans/. W ten sposób tylko "zapominasz" wszystkie dane z tab
    // czyli tab_trans[k] = new int[x];
}

for (int i = 0; i < y; i++)
{
    for (int j = 0; j < x; j++)
        tab_trans[i][j] = tab[j][i]; //tu wywala - bo tab_trans[i] nigdy nie miało przypisanej wartości (i są tam jakieś śmieci).
}

?

Bingo, głupia literówka, zamiast tab_trans miałem w kodzie tab, a tyle kombinowania. Mimo wszystko, dobrze, że zwróciłem się z pomocą, gdyż zwróciliście mi uwagę na te wycieki pamięci. Serdeczne dzięki!

Nie ma sensu zakładać nowego tematu, a jedna rzecz mnie jeszcze interesuje przy okazji rozwijania mojej klasy.
Otóż chcę zrobić funkcjonalność dodawania macierzy, za pomocą przeciążenia operatora +, np:

macierz A, B, C;
C = A + B 

Dodawać macierze można tylko gdy mają ten sam rozmiar. Ciekawi mnie, czy w przypadku gdy macierze będą miały różne wymiary, to zamiast w metodzie dać ifa sprawdzającego czy mają taki sam rozmiar i wypisującego np. tylko że nie ma możliwości dodania tych macierzy, można zrobić taki mechanizm, że program się w ogóle nie skompiluje, kompilator wyrzuci błąd, iż nie można dodawać macierzy o różnych wymiarach. Analogicznie kompilator protestuje, gdy będziemy chcieli dodać do siebie intigera i np. double'a. Da się coś takiego zrobić w C++?

Potrzebujesz następujących rzeczy:

Macierz operator+(const Macierz &m)const { ... }
Macierz &operator=(const Macierz &m) { ... }
Macierz(const Macierz &m) { ... }

oraz jeżeli to C++11 zastanów się nad:

Macierz &operator=(Macierz &&m) { ... }
Macierz(Macierz &&m) { ... }

Ciekawi mnie, czy w przypadku gdy macierze będą miały różne wymiary, to zamiast w metodzie dać ifa sprawdzającego czy mają taki sam rozmiar i wypisującego np. tylko że nie ma możliwości dodania tych macierzy, można zrobić taki mechanizm, że program się w ogóle nie skompiluje, kompilator wyrzuci błąd, iż nie można dodawać macierzy o różnych wymiarach

Można, tylko musiałbyś poczytać trochę o szablonach w C++ (http://en.wikipedia.org/wiki/Template_%28C%2B%2B%29)
Wtedy wyglądałoby to tak że można dodać Macierz<A, B> tylko do Macierz<A, B> (gdzie A, B to parametry określające wysokośc i szerokość). Ogólnie ciekawa rzecz do napisania, polecam.

A jakieś informację gdzie można o tym znaleźć? Ew. jak fachowo nazywa się taki zabieg?

Szablony, po prostu szablony.

Pierwszy (jeden z pierwszych) kurs z brzegu - http://www.tutorialspoint.com/cplusplus/cpp_templates.htm. To dość podstawy, ale wystarczyłoby.

Ale jeśli chcesz zepsuć sobie zabawę, sample (bardzo niekompletny ofc, i pamięć dla tab nie jest nigdy zwalniana ;) ):

template <int X, int Y>
struct Matrix {
    int **tab;

    Matrix<X, Y>() {
        tab = new int*[X];
        for (int i = 0; i < X; i++) {
            tab[i] = new int[Y];
        }
    }

    Matrix<X, Y> operator +=(Matrix<X, Y> other) {
        // add
    }
};

int main() {
    Matrix<2, 3> m;
    Matrix<2, 3> n;
    Matrix<5, 5> o;

    m += n;
    n += m;
    // n += o; - kompilator głośno krzyczy
}

Przy czym ważne jest zrozumieć że w tym przypadku jest problem z wczytywaniem macierzy o nieznanych rozmiarach (bo kompilator musi znać rozmiar każdej macierzy na etapie kompilacji żeby móc ja zweryfikować).

@msm, przekombinowałeś, jeżeli już masz rozmiary ustawione w parametrach to:

template<size_t X,size_t Y> struct Matrix
  {
   double tab[Y][X];
   Matrix() { memset(&tab[0][0],0,sizeof(tab)); }
   Matrix &operator+=(const Matrix &other)
     {
       // add
     }
  };