Funkcje - pierwsze starcie || Optymalizacja programu
Ostatnio zmodyfikowano 2016-04-04 22:06
Sirdis Temat założony przez niniejszego użytkownika |
Funkcje - pierwsze starcie || Optymalizacja programu » 2016-04-04 18:59:18 Witam wszystkich, napisałem kalkulator z działu funkcje pierwsze starcie. Wygląda na to, że działa poprawnie, chyba że coś przeoczyłem. Czy możecie spojrzeć na kod i powiedzieć czy tak to powinno wyglądać, czy gdzieś można go jakoś lepiej zoptymalizować? A może coś jest rzeczą karygodną i w ten sposób nie powinno się tego pisać? #include<iostream>
using namespace std;
int wczytywanieA()
{
int A;
bool test;
do
{
cout << "Podaj A: " << endl;
cin >> A;
test = cin.good();
if( test == 0 )
{
cout << "Blad, nie wprowadziles liczby" << endl;
cout << "Sprobuj ponownie" << endl;
}
cin.clear();
cin.ignore( 1000, '\n' );
} while( test == 0 );
return A;
}
int wczytywanieB()
{
int B;
bool test;
do
{
cout << "Podaj B: " << endl;
cin >> B;
test = cin.good();
if( test == 0 )
{
cout << "Blad, nie wprowadziles liczby" << endl;
cout << "Sprobuj ponownie" << endl;
}
cin.clear();
cin.ignore( 1000, '\n' );
} while( test == 0 );
return B;
}
int menu()
{
cout << "---------------------------------" << endl;
cout << "Witaj w prostym kalkulatorze v1.1" << endl;
cout << "---------------------------------" << endl;
cout << "Wybierz interesujace Cie dzialanie:" << endl;
cout << "[1] Dodawanie" << endl;
cout << "[2] Odejmowanie" << endl;
cout << "[3] Mnozenie" << endl;
cout << "[4] Dzielenie" << endl;
cout << endl;
cout << "[5] Zamknij program" << endl;
}
int dodawanie()
{
cout << "Wybrales dodawanie" << endl;
int A = wczytywanieA();
int B = wczytywanieB();
A + B;
return A + B;
}
int odejmowanie()
{
cout << "Wybrales odejmowanie" << endl;
int A = wczytywanieA();
int B = wczytywanieB();
A - B;
return A - B;
}
int mnozenie()
{
cout << "Wybrales mnozenie" << endl;
int A = wczytywanieA();
int B = wczytywanieB();
A * B;
return A * B;
}
int dzielenie()
{
cout << "Wybrales dzielenie" << endl;
int A = wczytywanieA();
int B = wczytywanieB();
A / B;
return A / B;
}
int main()
{
int liczba;
do
{
menu();
cin >> liczba;
if( liczba == 1 )
{
cout << "Twoj wynik to: " << dodawanie() << endl;
}
if( liczba == 2 )
{
cout << "Twoj wynik to: " << odejmowanie() << endl;
}
if( liczba == 3 )
{
cout << "Twoj wynik to: " << mnozenie() << endl;
}
if( liczba == 4 )
{
cout << "Twoj wynik to: " << dzielenie() << endl;
}
if( liczba == 5 )
{
cout << "Dziekuje!" << endl;
}
if( liczba != 1 && liczba != 2 && liczba != 3 && liczba != 4 )
{
cout << "Podaj prawidlowa liczbe" << endl;
cout << endl;
}
cin.clear();
cin.sync();
} while( liczba != 5 );
return 0;
}
|
|
michal11 |
» 2016-04-04 19:18:46 Czym się w zasadzie różnią funkcje wczytywanieA() i wczytywanieB() ? Mam na myśli działanie a nie nazwy zmiennych czy wyświetlany tekst. Jeżeli niczym to po co robić 2 praktycznie takie same funkcje ?
Nie sprawdzasz czy dzielisz przez zero.
Jeżeli już robisz menu na ifach to lepsze będzie else if wtedy nie masz niepotrzebnego sprawdzania (w najgorszym przypadku) 5 kolejnych warunków.
W każdej funkcji obliczającej przed returnem masz wykonane działanie, po co ? Skoro nie zapisujesz nigdzie jego wyniku to ta linijka nie ma sensu, puste wykonanie operacji którą zresztą za chwile znowu wykonujesz.
Ogólnie nie podobają mi się też prawie takie same funkcje na każde działanie ale rozumiem, że jest to program na przećwiczenie funkcji więc tego się nie czepiam, taka rada na przyszłość.
Aha no i nie należy w funkcjach dawać jednocześnie wykonywania jakiegoś działania i wyświetlania jakichś wyników itp. rzeczy, w funkcjach nie powinno być interfejsu użytkownika (nie licząc funkcji specjalnie do tego przygotowanych np. menu()). |
|
Sirdis Temat założony przez niniejszego użytkownika |
» 2016-04-04 20:26:49 Ok, dzięki za szybką reakcję i podpowiedzi. Na takie właśnie rady liczyłem. Postarałem się zastosować do wszystkich i rzeczywiście kod wygląda lepiej. Czy coś jeszcze powinienem zmienić? Chciałbym nauczyć się dobrej praktyki już na początku, żeby później się nie męczyć. #include<iostream>
using namespace std;
int wczytywanie()
{
int wartosc;
bool test;
do
{
cin >> wartosc;
test = cin.good();
if( test == 0 )
{
cout << "Blad, nie wprowadziles liczby" << endl;
cout << "Sprobuj ponownie" << endl;
}
cin.clear();
cin.ignore( 1000, '\n' );
} while( test == 0 );
return wartosc;
}
int menu()
{
cout << "---------------------------------" << endl;
cout << "Witaj w prostym kalkulatorze v1.3" << endl;
cout << "---------------------------------" << endl;
cout << "Wybierz interesujace Cie dzialanie:" << endl;
cout << "[1] Dodawanie" << endl;
cout << "[2] Odejmowanie" << endl;
cout << "[3] Mnozenie" << endl;
cout << "[4] Dzielenie" << endl;
cout << endl;
cout << "[5] Zamknij program" << endl;
}
int main()
{
int liczba;
do
{
menu();
cin >> liczba;
if( liczba == 1 )
{
cout << "Wybrales dodawanie." << endl;
cout << "Podaj liczbe A :" << endl;
int A = wczytywanie();
cout << "Podaj liczbe B :" << endl;
int B = wczytywanie();
cout << "Twoj wynik to: " << A + B << endl;
}
else
if( liczba == 2 )
{
cout << "Wybrales odejmowanie." << endl;
cout << "Podaj liczbe A :" << endl;
int A = wczytywanie();
cout << "Podaj liczbe B :" << endl;
int B = wczytywanie();
cout << "Twoj wynik to: " << A - B << endl;
}
else
if( liczba == 3 )
{
cout << "Wybrales mnozenie." << endl;
cout << "Podaj liczbe A :" << endl;
int A = wczytywanie();
cout << "Podaj liczbe B :" << endl;
int B = wczytywanie();
cout << "Twoj wynik to: " << A * B << endl;
}
else
if( liczba == 4 )
{
cout << "Wybrales dzielenie." << endl;
cout << "Podaj liczbe A :" << endl;
int A = wczytywanie();
int B;
do
{
cout << "Podaj liczbe B :" << endl;
B = wczytywanie();
if( B == 0 )
{
cout << "Blad, nie dzielimy przez 0!" << endl;
}
else;
} while( B == 0 );
cout << "Twoj wynik to: " << A / B << endl;
}
else
if( liczba == 5 )
{
cout << "Dziekuje!" << endl;
}
else
cout << "Podaj prawidlowa liczbe" << endl;
cout << endl;
} while( liczba != 5 );
return 0;
}
|
|
michal11 |
» 2016-04-04 20:47:23 Moim zdaniem teraz masz za dużo kodu, szczególnie w ifach już lepszy był pomysł z oddzielnymi funkcjami, teraz jest tylko bałagan w main. To co ja pisałem o prawie takich samych funkcjach na każde działanie, to chodziło mi o to, że ja bym zupełnie inaczej zaprojektował ten program, ogólnie poprzednie podejście z funkcjami było lepsze niż to co masz teraz. Spróbuj skorzystać z argumentów funkcji np. int wczytywanie( char nazwaZmiennej )
{
int wartosc;
bool test;
do
{
cout << "Podaj " << nazwaZmiennej << ": " << endl;
cin >> wartosc;
test = cin.good();
if( test == 0 )
{
cout << "Blad, nie wprowadziles liczby" << endl;
cout << "Sprobuj ponownie" << endl;
}
cin.clear();
cin.ignore( 1000, '\n' );
} while( test == 0 );
return wartosc;
}
I nie porównuj booli z liczbami, po to wprowadzono true i false żeby nie było takich właśnie magicznych liczb. |
|
Sirdis Temat założony przez niniejszego użytkownika |
» 2016-04-04 21:55:49 hmmm teraz się trochę pogubiłem. Rozumiem co masz na myśli z przenoszeniem wartości do funkcji. Nie wiem jednak jak przenieść argument w postaci znaku do funkcji która wypisze ten znak. Trochę pokręciłem. Mam pomysł jak ten program skrócić ale jeszcze nie potrafię tego zrobić. Chyba potrzebuję więcej czasu na to. |
|
michal11 |
» 2016-04-04 22:06:44 To co masz jest moim zdaniem na ta chwilę ok. Zamień tylko t co masz w ifach na funkcje tak jak było i będzie dobrze. Nie trać na to za dużo czasu tylko rób kolejne lekcje. |
|
| « 1 » |