Program - działania na macierzach

0

Witam, jako projekt semestralny mam napisać program o tematyce dowolnej, musi być napisany obiektowo, zawierać przeciążane operatory i dziedziczenie. Mój pomysł to działania na macierzach. Po kilku(może nawet nasto) godzinnej walce mam coś takiego:

#include <iostream> 
using namespace std; 
 
class macierz 
{ 
      public: 
      //zmienne
      int wiersze;
      int kolumny;  
      double **tablica;        //wskaznik na dynamiczna tablice (macierz) 
      
      //konstruktory i destruktory
      macierz(int wiersze, int kolumny); //konstruktor dwuparametrowy
      macierz(); //konstruktor domyslny 
      ~macierz(){delete []tablica;} //destruktor domyslny
      macierz(macierz const &m); //konstruktor kopiujacy 
      
      //zaprzyjaznione operatory przeciazane
      friend macierz operator+(const macierz &m1, const macierz &m2); 
      friend macierz operator-(const macierz &m1, const macierz &m2);
      friend macierz operator*(const macierz &m1, const macierz &m2);
      friend macierz operator*(const macierz &m1, const double skalar); 
      //metody
      void Wyswietl()const; 
}; 
///////////////////////////////////////////////////////////////////////////////
macierz::macierz ()  //domyslnie tworzy macierz 2x2 wypelniona zerami
{          
      wiersze=kolumny=2;
       
      tablica = new double *[wiersze]; 
      for (int i=0; i<wiersze;i++) 
          tablica[i]=new double[kolumny];    
       
      for ( int i = 0; i < kolumny; i ++ ) 
      { 
          cout<<"wiersz "<<i<<": "; 
          for (int j=0; j<wiersze; j++)  
              tablica[i][j] = 0; 
        
      } 
      Wyswietl();  
} 
//////////////////////////////////////////////////////////////////////////////// 
 
macierz::macierz(macierz const &m)  //konstruktor kopiujacy
{ 
    kolumny = m.kolumny; 
    wiersze = m.wiersze; 
    tablica = new double *[wiersze]; 
    for (int i = 0; i < wiersze; i ++ ) 
        tablica[i] = new double[kolumny]; 
     
    for (int i = 0; i < wiersze; i ++ ) 
        for (int j = 0; j < kolumny; j ++ ) 
            tablica[i][j] = m.tablica[i][j]; 
} 
 
///////////////////////////////////////////////////////////////////////////////
macierz::macierz (int wier, int kol)  //konstruktor dwuparametrowy
{ 
      kolumny = kol; 
      wiersze = wier; 
      double wartosc; 
      tablica = new double *[wiersze]; 
      for (int i=0; i<wiersze;i++) 
          tablica[i]=new double[kolumny];    
       
      for ( int i = 0; i < wiersze; i ++ ) 
      {  for (int j=0; j<kolumny; j++) 
          { cout << "Wprowadz [" << i << "]" << "[" << j << "] element macierzy: ";
              cin>>wartosc; 
              tablica[i][j] = wartosc; 
          } 
      }  
} 
///////////////////////////////////////////////////////////////////////////////
void macierz::Wyswietl()const 
{ 
      for ( int i = 0; i < wiersze; i ++ ) 
      { 
          cout<<endl<<"wiersz "<<i<<": \t"; 
          for (int j=0; j<kolumny; j++) 
          { 
              cout<<tablica[i][j]<<" "; 
          } 
      } 
      cout<<endl;  
} 
//////////////////////////////////////////////////////////////////////////////
macierz::macierz operator+(const macierz &m1, const macierz &m2) 
{ 
macierz m3(m1); 
  if (m1.wiersze == m2.wiersze && m1.kolumny == m2.kolumny)  
  { 
      for (int i=0; i<m3.wiersze; i++) 
          for (int j=0; j<m3.kolumny; j++) 
          { 
              m3.tablica[i][j] += m2.tablica[i][j]; 
          } 
  } 
  else 
      cout<<"Nie mozna dodac macierzy!!\n"; 
  return m3; 
} 
////////////////////////////////////////////////////////////////////////////////
macierz::macierz operator-(const macierz &m1, const macierz &m2) 
{ 
macierz m3(m1); 
  if (m1.wiersze == m2.wiersze && m1.kolumny == m2.kolumny)  
  { 
      for (int i=0; i<m3.wiersze; i++) 
          for (int j=0; j<m3.kolumny; j++) 
          { 
              m3.tablica[i][j] -= m2.tablica[i][j]; 
              //m3.tab[i][j]=m1[i][j]+m2[i][j]     
          } 
  } 
  else 
      cout<<"Nie mozna odjac macierzy!!\n"; 
  return m3; 
}
///////////////////////////////////////////////////////////////////////////////////
macierz::macierz operator*(const macierz &m1, const macierz &m2) 
{ 
macierz m3(m1); 
  if (m1.kolumny == m2.wiersze)  
  { 
       for (int i = 0; i <m1.wiersze; i++)        
       for (int j = 0; j <m2.kolumny; j++) 
       m3.tablica[i][j] = 0;            
       for(int i = 0; i < m1.wiersze; i++)
       for(int j = 0; j < m2.kolumny; j++)
       for(int k = 0; k < m2.wiersze; k++)
       m3.tablica[i][j] = m3.tablica[i][j] + m1.tablica[i][k] * m2.tablica[k][j];
  } 
  else 
      cout<<"Nie mozna pomnozyc macierzy!!\n"; 
  return m3; 
}  

//////////////////////////////////////////////////////////////////////////
macierz::macierz operator*(const macierz &m1, const double skalar) 
{ 
macierz m2(m1); 
  
       for (int i = 0; i <m1.wiersze; i++)        
       for (int j = 0; j <m1.kolumny; j++) 
       m2.tablica[i][j] = m1.tablica[i][j]*skalar;            
       return m2; 
}  

//////////////////////////////////////////////////////////////////////
void wyswietlmenu(){
    cout << "Program wykonujacy dzialania na macierzach\n";
    cout << "1.Dodaj macierze\n";
    cout << "2.Odejmij macierze\n";
    cout << "3.Pomnoz macierze (A*B)\n";
    cout << "4.Pomnoz przez skalar\n";
    cout << "5.Transponuj (tylko dla macierzy kwadratowej)\n";
    cout << "6.Wyjscie\n";
    cout << "Wybierz wlasciwa opcje\n";
     };
       
 
 
/////////////////////////////////////////////////////////////////// 
int main() 
{   wyswietlmenu();
    int wybor;
    cin >> wybor;
    int liczbawierszy;
    int liczbakolumn;
    
    switch (wybor){

           case 1: {cout << "Wprowadz liczbe wierszy pierwszej macierzy: ";
                   cin >> liczbawierszy;
                   cout << "Wprowadz liczbe kolumn pierwszej macierzy: ";
                   cin >> liczbakolumn;
                   macierz A (liczbawierszy,liczbakolumn);
                   cout << "Wprowadz liczbe wierszy drugiej macierzy: ";
                   cin >> liczbawierszy;
                   cout << "Wprowadz liczbe kolumn drugiej macierzy: ";
                   cin >> liczbakolumn;
                   macierz B (liczbawierszy,liczbakolumn);
                   macierz C=A+(B); 
                   C.Wyswietl(); break;}
                   //DODAWANIE

           case 2: {cout << "Wprowadz liczbe wierszy pierwszej macierzy: ";
                   cin >> liczbawierszy;
                   cout << "Wprowadz liczbe kolumn pierwszej macierzy: ";
                   cin >> liczbakolumn;
                   macierz A (liczbawierszy,liczbakolumn);
                   cout << "Wprowadz liczbe wierszy drugiej macierzy: ";
                   cin >> liczbawierszy;
                   cout << "Wprowadz liczbe kolumn drugiej macierzy: ";
                   cin >> liczbakolumn;
                   macierz B (liczbawierszy,liczbakolumn);
                   macierz C=A-(B); 
                   C.Wyswietl(); break;}//ODEJMOWANIE
                   
           case 3: {cout << "Wprowadz liczbe wierszy pierwszej macierzy: ";
                   cin >> liczbawierszy;
                   cout << "Wprowadz liczbe kolumn pierwszej macierzy: ";
                   cin >> liczbakolumn;
                   macierz A (liczbawierszy,liczbakolumn);
                   cout << "Wprowadz liczbe wierszy drugiej macierzy: ";
                   cin >> liczbawierszy;
                   cout << "Wprowadz liczbe kolumn drugiej macierzy: ";
                   cin >> liczbakolumn;
                   macierz B (liczbawierszy,liczbakolumn);
                   macierz C=A*(B); 
                   C.Wyswietl(); 
                   break;} //MNOZENIE

           case 4: {cout << "Wprowadz liczbe wierszy pierwszej macierzy: ";
                   cin >> liczbawierszy;
                   cout << "Wprowadz liczbe kolumn pierwszej macierzy: ";
                   cin >> liczbakolumn;
                   macierz A (liczbawierszy,liczbakolumn);
                   cout << "Wprowadz skalar: ";
                   double skalar;
                   cin >> skalar;
                   macierz C=A*(skalar);
                   C.Wyswietl();
                   break;}
                ; //MNOZENIE PRZEZ SKALAR
           case 5:break; //TRANSPONOWANIE
           case 6:break; //WYJSCIE
           default:break  ;
};
    system ("PAUSE");
    return 0; 
} 

Wiem, że można by to napisać bardziej elegancko, uzyc plikow naglowkowych itd. ale nie o to mi chodzi. chcĘ zeby ktos rzucil okiem na kod i wytknal ewentualne bledy wraz ze sposobem ich poprawy ze szczegolnym naciskiem na przeciazanie operatorow, alokowanie i zwalnianie pamieci jako, ze tego dopiero sie ucze. Zaznaczam, ze program nie jest skonczony bo mam zamiar jeszcze stworzyc klase pochodna macierzy kwadratowej i klase pochodna macierzy jednostkowej. Jest to dopiero szkielet ale chciałbym po prostu wiedziec czy zmierzam w dobrym kierunku. Pozdrawiam, Piotr.

1
Piotr napisał(a)

Wiem, że można by to napisać bardziej elegancko, uzyc plikow naglowkowych itd. ale nie o to mi chodzi. chcĘ zeby ktos rzucil okiem na kod i wytknal ewentualne bledy wraz ze sposobem ich poprawy ze szczegolnym naciskiem na przeciazanie operatorow, alokowanie i zwalnianie pamieci jako, ze tego dopiero sie ucze. Zaznaczam, ze program nie jest skonczony bo mam zamiar jeszcze stworzyc klase pochodna macierzy kwadratowej i klase pochodna macierzy jednostkowej. Jest to dopiero szkielet ale chciałbym po prostu wiedziec czy zmierzam w dobrym kierunku. Pozdrawiam, Piotr.

po pierwsze primo, nie wiem jakiego kompilatora uzywasz, mi program nie kompiluje się na vs2008:)

linijki w stylu:

macierz::macierz operator+(const macierz &m1, const macierz &m2) 

nie są poprawne, ponieważ dwuargumentowy operator+ powinien zwracac po prostu "macierz", a nie "macierz::macierz" ktore nie wiadomo czym jest. Zakladam ze jest to literowka powstala gdzies przy kopiowaniu kodu.. po usunieciu temu podobnych program sie kompilowal.

dalej: pomysl na macierz i pochodne: kwadratowa, jednostkowa jest bardzo dobry. IMHO całkowicie spełnia LSP i jednocześnie jest sensowny gdyz przynajmniej m.jednostkowa daje popis na szczegolne specjalizace. M.Kwadratowa - szczerze, nie wiem, chyba prędzej pasmowa np. n-diagonalna, albo rzadka.. ale to malo istotne.

błąd#000:
w konstruktorze domyslnym, kopiujacym, oraz tym dwuargumentowym masz "tablica = new double *[wiersze] / tablica[i]=new double[kolumny];", to znaczy ze Twoj destruktor zwalnia tylko czesc pamieci ktora alokujesz w nich

dziw#000:
konstruktor domyslny cos wyswietla, a inne tego nie robia.
warto tez zauwazyc, ze nawet jesli tam chcesz cos wypisac, to masz cout<<"wiersze" podczas gdy I jest w forze az do "kolumny"

komentarz#000:
we wszystkich konstruktorach masz uklad petla-cos, potem petla-petla-cos. zewnetrzne petle sa identyczne, kody w srodku dotykaja tylko "aktualnych" indeksow, wiec mozesz je smialo scalic w jedna petle.
plusy: przy dwoch petlach trudniej o blad czy literowke niz przy trzech
minusy: aktualny kod tez jest troche czytelniejszy niz by byl przy dwoch

dziw#001:
domyslny ctor wywoluje funkcje Wyswietl(), a inne tego nie robia.
generalnie ctor'y powinny zajmowac sie przygotowaniami. To, ze gdzies-tam w programie chciales przy okazji te macierz wyswietlic, nie powinno żadnego ctor'a interesowac! po prostu w tamtym miejscu programu wywolaj te metode

komentarz#001:
w ctor'ze dwuparametrowym korzystasz ze zmiennej "double wartosc". jej "czas życia" jest zbyt dlugi i mozesz, nie tutaj, ale kiedys, przy wiekszych/dluzszych funkcjach przez takie cos miec jakies trudne do wylapania bledy. tutaj wszystko jest poprawnie, ale sprobuj sie dla swietego spokoju przyzwyczaic, zeby zmiennym ograniczac widocznosc, zwlaszcza te prostym - bo to nic nie kosztuje. Definicja zmiennej "double wartosc" powinna byc gleboko w najnizszej petli, np. tuz przed cin<<wartosc. Bez obaw, kompilator nie jest tak glupi zeby ciagle ja alokowac i dealokowac na stosie. Jedyne przypadki ktore mi teraz przychodza na mysl w ktorych jest faktycznie sens wyciagac taka zmienna przed petle to:

  • rzeczywiscie potrzebujesz po skonczonych petlach znac ostatnia wartosc tej zmiennej jaka sie w petlach pojawila
  • typ tej zmiennej ma konstruktor/destruktor, być może ciężki, a Ty chcesz zaoszczedzic "czas" na ich nieodpalaniu co obieg pętli
    ani jedno, ani drugie nie zachodzi przy tej zmiennej.
    ta sama uwaga odnosi sie do "liczbywierszy/kolumn" w main'ie

dziw#002:
tak w ogole, czemu ctor 2arg'owy w ogóle pobiera dane od uzytkownika? zaden inny Twoj ctor tego nie robi. Copy-ctor to jasne, ale czemu na przyklad ctor domyslny ktory przygotowyje 2x2 nie pyta sie o dane? jest to niespojne. Zamiast tutaj ladowac dane z "klawiatury", przygotuj metode "pobierzDaneOdUzytkownika" lub cos w ten desen i tam to zrob.

bug#001:
pobierasz dane poprzez cin>>wartosc, ale nie sprawdzasz czy sie udalo je odczytac. co bedzie jesli uzytkownik wpisze "alamakota"? sprawdzaj cin.fail() po kazdym odczycie i obsluż jakoś fakt, że user może po prostu nie trafic w klawisz czasem:) obsluga bledow jest upierdliwa, i tym bardziej wskazuje ze ten kod "interaktywnego" czytania ze strumienia bedzie sporo wiekszy i ze nie powinien byc tutaj w ctorze (z drugiej storny, jesli byloby to czytanie NIEinteraktywne, np. z pliku - to wtedy jak najbardziej, ale ctor wtedy powinien brzmiec np (istream& input, int wiersz, int kol) a nie na sztywno z cin.
Ten sam problem masz w main, przy cin>>wybor, acz tam tego nie widac az tak. Sprobuj jednak menu "zapetlic" aby mozna bylo wybrac opcje jeden a potem opcje dwa, a okaze sie ze dzieja sie bardzo brzydkie rzeczy jesli ktos w menu wcisnie "Q" zamiast "6"

komentarz#002:
no wlasnie.. int wiersz/int kol - czemu INT? one moga byc ujemne? jesli nie, to uzywaj unsigned int, unsigned long, albo w skrocie: size_t

komentarz#003:
ujemne-ujemnymi, ale na "zerowe" tez trzeba uwazac. Jesli Twoja macierz ma z "definicji" niedopuszczac na bycie macierzą np. 5x0, to ctory powinny jakos to sprawdzac i obslugiwac lub rzucac wyjatkami

komentarz#004:
pierdółka: pewnie jakies miejsce w Twoim programie potrzebowalo lamania linii takiego a nie innego, ale prosciej by ci bylo zorganizowac to samo tak:

      for ( int i = 0; i < wiersze; i ++ ) 
      { 
          cout<<"wiersz "<<i<<": \t"; 
          for (int j=0; j<kolumny; j++) 
              cout<<tablica[i][j]<<" "; 
          cout<<endl;  
      } 

i pozostawic kwestię rozpoczynania od nowego wiersza osobie wywołującej. ale to jest kwestia "gustu"

komentarz#005:
wszystkie operatory+/-/* najpierw tworzą kopię macierzy, a potem sprawdzają czy to ma sens. em.. lepiej odwrotnie, zaoszczędzisz trochę czasu i roboty programowi, jesli najpierw sprawdzisz a potem dopiero nakażesz pracować. Linia "macierz m3(m1);" wyglada niewinnie, ale tam sie copyctor odpala i puste kopiowanie macierzy 1000x1000 juz sensowne nie jest!

bug#002:
ano, bug.. chociaz widac ze raczej po prostu nie miales pomyslu co z tym fantem zrobic..
operatory +/-/* w razie porażki wypisują na cout komunikat, a program leci dalej.
na pewno chcesz w przypadku obliczania wyrazenia Y = dAX - fEH, jesli pojawi sie zonk że wymiary AX!=EH, zwracać Y = dAH? IMHO tak nie powinno byc.. jesli juz taka operacje "przyjaznego dodawania" chcesz miec, nie powinien to byc operator+ ktory tradycyjniie gwarantuje ze wynik będzie sumą argumentów. To co powinienes zrobic w przypadku wykrycia "złej sytuacji", to rzucenie wyjątku, albo zwrócenie specjalnej macierzy-błędu, np w całości wypełnionej wartościami double NaN (not-a-number), np.:

macierz operator+(const macierz &m1, const macierz &m2) 
{ 
  if (!(m1.wiersze == m2.wiersze && m1.kolumny == m2.kolumny))
    throw std::runtime_error("Wymiary macierzy nie sa zgodne, nie mozna wykonac dodawania");

  macierz m3(m1); 
  for (int i=0; i<m3.wiersze; i++) 
    for (int j=0; j<m3.kolumny; j++) 
      m3.tablica[i][j] += m2.tablica[i][j]; 

  return m3; 
} 

mały bonus - zwroc uwage, ze taki układ "sterowania" jest lekko czytelniejszy (mniej zagniezdzen klamer) i automatycznie rozwiazuje komentarz#005
mały bonus - albo minus, jak dla kogo - zmusza Cie to do o-try-catch-owania programu i zastanowienia sie, jak w ogole obslugiwac bledy, i kiedy i jak je wypisywac/raportowac/logowac/... ale zwroc tez uwage, ze u Ciebie wystarczy jeden albo kilka try-catch'y wokół switch albo case w main!

bug#003:
operator mnozenia macierzy jest IMHO błędny, poniewaz rozpoczynasz go z macierza wynikowa o wymiarach równych do macierzy "lewej". Z tego co pamietam, mnożenie macierzy bierze N wierszy z lewej a M kolumn z prawej. Oczywiscie, masz tam warunek m1.kolumny == m2.wiersze ktory chroni Twoje petle przed wyjsciem poza "tablice", ale nie tedy droga. Po porstu trzeba bylo macierz M3 skonstruowac jako m1.wierszyXm2.kolumn. Pewnie nie mogles tego zrobic jako ze Twoj ctor dwuargumentowy pobieral dane od uzytkownika? o, i kolejny przyklad ze pobieranie danych tam nie powinno bylo w ogole siedziec!

dziw#004:
zrobiles "wyswietlmenu", a chcialo Ci sie w main'ie kopiuj-wklej'owac ciagle pytania o liczbe wierszy/kolumn? wez zrob na to jedna albo dwie funkcyjny, bo az w oczy kluje.. np. plan minimum:

size_t pobierz(string czego, string dlakogo)
{
  cout << "Wprowadz liczbe "<< czego <<" "<< dlakogo <<" macierzy: ";
  cin >> liczbawierszy;   // <--- oczywiscie ewentualny blad CIN'a jakos warto obsluzyc
  return liczbawierszy;
}

....
case 1:
{
  macierz A (  pobierz("wierszy", "pierwszej"),  pobierz("kolumn", "pierwszej"));
  A.wypytajIUstawElementyWgCheciUzytkownika();

  macierz B (  pobierz("wierszy", "drugiej"),  pobierz("kolumn", "drugiej"));
  B.wypytajIUstawElementyWgCheciUzytkownika();

  macierz C=A+(B); 
  C.Wyswietl();
  break;
}

a fakt, ze pobierz+pobierz+wypytajIUstawElementyWgCheciUzytkownika sie powtarzaja i ze pewnie wlasnie dlatego w ctor-2param dales czytanie elementow z klawiatury, sprawia, ze warto pomyslec czy w ogole czytania z macierzy nie zamknac w funkcji zwracajacej gotowa do pracy macierz:

macierz pobierz(string ktorej)
{
  macierz A (  pobierz("wierszy", ktorej),  pobierz("kolumn", ktorej));
  A.wypytajIUstawElementyWgCheciUzytkownika();
  return A;
}
...
case 1:
{
  macierz A = pobierz("pierwszej");
  macierz B = pobierz("drugiej");
  macierz C=A+(B); 
  C.Wyswietl();
  break;
}

ale to juz tez mozna powiedziec, to znowu stylistyka..

0

Przede wszystkim wielkie dzięki za zainteresowanie tematem i bardzo fachowe rady.
Więc teraz tak:

  • poprawiłem destruktor - teraz zwalnia całą zaalokowaną pamięć
  • zrobiłem funkcję obsługabledow, która odpowiada za utrzymanie poprawnosci danych wpisywanych przez uzytkownika
  • przerzucilem wprowadzanie wartosci z konstruktora do funkcji WprowadzDane
  • zmieniłem int na unsigned int
  • wrzucilem kopiowanie macierzy wewnatrz if'a dzieki czemu jest kopiowana tylko po spelnieniu warunkow
  • nie wiem natomiast czemu uwazasz ze operator mnozenia jest zly... Sprawdzalem rozne kombinacje wraz z dostepnym on-line kalkulatorem macierzowym i moj program dawal identyczne wyniki
  • caly czas najwiekszy problem mam problem z tym co zrobic, jezeli wymiary macierzy uniemozliwiaja wykonanie dzialania

(link w poscie nizej)

A i używam póki co DevC++ bo VS nie dam rady sciagnac na razie ze wzgledu na objetosc...

0

proszę cię, wrzuć to na pastebina albo coś tylko nie wklejaj tutaj takich długich kodów.

0

http://pastebin.com/bbVCeVBL
Jeśli robie coś nie tak to prosze o wyrozumialosc, pierwszy dzien jestem na tym forum...

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