Czy mozna to jeszcze jakos zoptymalizowac? (vector, funkcje)

Question

Witam chciałem, się dowiedzieć czy ten program jest dobrze napisany podwzględem czystości kodu, przejrzystości itd.
(Wiem ze deklaracje i definicje funkcji najlepiej było by rozbić na 2 różne pliki, pomińmy ten fakt w ocenie)

Nie wiedziałem kompletnie jak wmiare krótko ale dobrze nazwać parametry funkcji wczytywanie. Moze ktoś ma lepszy pomysł

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

void wczytywanie(unsigned int* liczbaWierszyA, unsigned int* liczbaKolumnA_liczbaWierszyB, unsigned int* liczbaKolumnB);
void uzupelnij(vector< vector<unsigned int> >& macierz, string nazwa_macierzy);
void wyswietl(vector< vector<unsigned int> >& macierz, string nazwa_macierzy);
void mnozenie(vector< vector<unsigned int> >& macierz1, vector< vector<unsigned int> >& macierz2, vector< vector<unsigned int> >& macierz3);

int main()
{
	unsigned int liczba_wierszy_A{}, liczba_kolumn_A_i_liczba_wierszyB{}, liczba_kolumn_B{};
	wczytywanie(&liczba_wierszy_A, &liczba_kolumn_A_i_liczba_wierszyB, &liczba_kolumn_B);

	vector< vector<unsigned int> > macierzA
	{
		liczba_wierszy_A,
		vector<unsigned int>(liczba_kolumn_A_i_liczba_wierszyB)
	};
	uzupelnij(macierzA, "A");
	wyswietl(macierzA, "A");

	vector< vector<unsigned int> > macierzB
	{
		liczba_kolumn_A_i_liczba_wierszyB,
		vector<unsigned int>(liczba_kolumn_B)
	};
	uzupelnij(macierzB, "B");
	wyswietl(macierzB, "B");

	vector< vector<unsigned int> > macierzC
	{
		liczba_wierszy_A,
		vector<unsigned int>(liczba_kolumn_B)
	};
	mnozenie(macierzA, macierzB, macierzC);
	wyswietl(macierzC, "wynikowej C");

	system("PAUSE");
}
void wczytywanie(unsigned int* liczbaWierszyA, unsigned int* liczbaKolumnA_liczbaWierszyB, unsigned int* liczbaKolumnB)
{
	cout << "Okresl wielkosc macierzy!" << endl;
	do
	{
		cout << "Podaj ile wierszy ma miec macierz A: ";
		cin >> *liczbaWierszyA;
	} while (*liczbaWierszyA == 0);
	do
	{
		cout << "Podaj liczbe kolumn macierzy A, wierszy macierzy B: ";
		cin >> *liczbaKolumnA_liczbaWierszyB;
	} while (*liczbaKolumnA_liczbaWierszyB == 0);
	do
	{
		cout << "Podaj liczbe kolumn macierzy B: ";
		cin >> *liczbaKolumnB;
	} while (*liczbaKolumnB == 0);
	cout << endl;
}
void uzupelnij(vector< vector<unsigned int> >& macierz, string nazwa_macierzy)
{
	cout << "Uzupelnij macierz " + nazwa_macierzy + " wartosciami!" << endl;
	for (int i = 0; i < macierz.size(); i++)
	{
		for (int j = 0; j < macierz[i].size(); j++)
		{
			cout << "Do elementu o indeksie [" << i << "][" << j << "] wpisz: ";
			cin >> macierz[i][j];
		}
	}
	cout << endl;
}
void wyswietl(vector< vector<unsigned int> >& macierz, string nazwa_macierzy)
{
	cout << "Wyswietlenie macierzy " + nazwa_macierzy + "!" << endl;
	for (int i = 0; i < macierz.size(); i++)
	{
		for (int j = 0; j < macierz[i].size(); j++)
		{
			cout << macierz[i][j] << "\t";
		}
		cout << endl;
	}
	cout << endl;
}
void mnozenie(vector< vector<unsigned int> >& macierz1, vector< vector<unsigned int> >& macierz2, vector< vector<unsigned int> >& macierz3)
{
	for (int i = 0; i < macierz1.size(); i++)
	{
		for (int j = 0; j < macierz2[i].size(); j++)
		{
			for (int k = 0; k < macierz2.size(); k++)
			{
				macierz3[i][j] += macierz1[i][k] * macierz2[k][j];
			}
		}
	}
	cout << endl;
}

 

Comments

void wczytywanie(unsigned int*, unsigned int*, unsigned int* );

Dlaczego używasz wskaźników tam, gdzie spokojnie można użyć (bezpieczniejszych) referencji?

Zamiast std::endl używaj '\n' (no chyba że koniecznie chcesz flushować strumień).

---

Z tego co wiem, to windows i linux domyślnie flushuje strumień po każdym wypisaniu czegokolwiek. Więc IMO w tym przypadku, to bez znaczenia.

---

Nie tyle flushuje, co nie buforuje (dlatego fushowanie jest bez sensu).

---

@j23, 1. Wskaźniki już zmieniłem na referencje w tej funkcji dzięki
2. Odnoścnie tego endl to nigdy niewiem czy co jest lepsze słyszałem że niby bez różnicy który zapis stosuje. Szukaj teraz trafiłem na ten post https://forum.pasja-informatyki.pl/307571/n-czy-endl i nwm czy dalej tak jest czy coś w C++17 sie zmieniło.

 

---

słyszałem że niby bez różnicy

Różnica jest taka, że endl flushuje bufor strumienia, nawet jeśli strumień nie jest buforowany, to i tak wywoływana jest metoda ostream::flush. W nowych standardach nic się pod tym względem nie zmieniło.

No i jest jeszcze aspekt czysto praktyczny. Zamiast zapisywać:

std::cout << "linia tekstu" << std::endl;

można krócej tak:

std::cout << "linia tekstu\n";

 

Answer 1

Kilka uwag odnośnie kodu.

1. Skracaj kod tam gdzie tylko się to jest możliwe. Jak użyjesz

   using Macierz = vector<vector<unsigned int>>;

   to wszędzie tam gdzie występuję vector<vector<unsigned int>> możesz zastapić to typem Macierz

2. Używaj const string& zamiast string w deklaracji/definicji swoich funkcji. Teraz za każdym ich wywołaniem tworzony jest niepotrzebnie nowy obiekt typu string.

3. Nie używaj wskaźników. Zastąp je referencjami lub zwracaj obiekt typu ntuple jak w przykładzie poniżej:

   #include <iostream>
   #include <vector>
   #include <tuple>
   
   using namespace std;
   
   // C++17
   
   auto get_int( const string& info )
   {
       int value{};
       while( value == 0 ){ cout << info; cin >> value; }
       return value;
   }
   
   auto load()
   {
       cout << "Okresl wielkosc macierzy!" << endl;
       return make_tuple( get_int("Podaj liczbe wierszy: "),
                          get_int("Podaj liczbe kolumn: ")   );
   }
   
   int main()
   {
       auto [wiersze,kolumny] = load();
       cout << wiersze << " : " << kolumny << endl;
   }

    

 

Comments

1. i 2. Faktycznie dzieki niepomyślałem o tym.
3. Jestem w trakcie nauki c++ 14/17 jeszcze nie spotakłem sie z biblioteką #include <tuple>, ale skoro przydała by się nawet w tym programie to poczytam sb o niej