Refaktoryzacja programu z c++

Question

Witam, chciałbym prosić o sprawdzenie czy można coś zapisać jeszcze krócej niż jest obecnie w programie (chodzi mi o refaktoryzację).

Program:

#include <iostream> // podstawowa biblioteka c++
#include <cstdlib> // do losowania liczba
#include <time.h> // do pobierania czasu z systemu
#include <stdio.h> // do getchar
#include <conio.h> // do automatycznego menu
#include <windows.h> // do odczekiwania, pobieranie czasu
using namespace std;
string wybor;  //menu
int przedmioty;int liczba;float oceny;int wynik=1;int kalkulator=0; // kalkulator sredniej
int liczba_losowa;int strzal;int ile_prob=0; // zgadnij jaka to liczba
int main()
{
    for(;;)
    {
        cout<<"Instrukcja:"<<endl;
        cout<<"1. Kalkulator sredniej z ocen"<<endl;
        cout<<"2. Gra w zgadnij jaka to liczba"<<endl;
        cout<<"3. Wylacz program"<<endl;
        cout<<"Podaj numer programu:"<<endl;
        cin>>wybor;
        if(wybor=="1"||wybor=="1.")
        {
            przedmioty=0;//reset
            liczba=0;
            oceny=0;
            wynik=1;
            kalkulator=0;//koniec resetu

            cout << "Wybrano program pierwszy"<<endl<<endl;
            cout << "Kalkulator sredniej z ocen"<<endl;
            cout << "Podaj liczbe ocen do sredniej:";
            cin >> przedmioty;
            if ((przedmioty>0))
            {
                while (kalkulator<przedmioty)
                {
                    cout << "Wpisz ocene:";
                    cin >> liczba;
                    if ((liczba==1)||(liczba==2)||(liczba==3)||(liczba==4)||(liczba==5)||(liczba==6))
                    {
                        if (wynik==przedmioty)
                        {
                            oceny=liczba+oceny;
                            cout << "Ilosc przedmiotow:"<< endl;
                            cout << przedmioty<< endl;
                            cout << "Suma twoich ocen:"<< endl;
                            cout << oceny<< endl;
                            cout << "Twoja srednia:"<< endl;
                            cout << oceny/przedmioty << endl;
                            cout << "Powodzenia w szkole :)" << endl;
                            kalkulator=kalkulator+1;
                            getchar();getchar();
                            system("cls");
                        }
                        else
                        {
                         oceny=liczba+oceny;
                         wynik=wynik+1;
                         kalkulator=kalkulator+1;
                        }
                    }
                    else
                    {
                        cout << "Wykryto nieprawidlowa liczbe, sprobuj jeszcze raz"<<endl;
                        kalkulator-1;
                    }
                }

            }
            else
            {
                cout << "Liczba przedmiotow musi byc wieksza od 0!";

            }
        }
        else if(wybor=="2"||wybor=="2.")
        {
            ile_prob=0; //reset

            cout << "Wybrano program drugi"<<endl<<endl;
            cout << "Witaj! Pomyslalem sobie liczbe od 1 do 100" << endl;
            srand(time(NULL));
            liczba_losowa=rand()%100+1;

            while(strzal!=liczba_losowa)
            {
                ile_prob++;
                cout<<"Sprobuj jaka to liczba:";
                cin>>strzal;
                if (strzal==liczba_losowa)
                {
                    cout<<"Brawo, udalo Ci sie odgadnac! Wygrywasz w: "<<ile_prob<<" probie!"<<endl;
                    getchar();getchar();
                    system("cls");
                }

                else if (strzal<liczba_losowa)
                {
                    cout<<"To za malo! (to byla twoja "<<ile_prob<<" proba)"<<endl;
                }

                else if (strzal>liczba_losowa)
                {
                    cout<<"To za duzo!(to byla twoja "<<ile_prob<<" proba)"<<endl;
                }
            }
        }
        else if(wybor=="3"||wybor=="3.")
            {
                exit(0);
            }
        else
        {
            cout << "Nieprawidlowy numer programu, sprobuj jeszcze raz za 3 sekundy"<<endl;
            Sleep(3000);
            system("cls");
        }
    }
    return 0;

} 

Comments

uzyj biblioteki random do losowania, iostream nie jest biblioteka podstawowa, nieuzywaj biblioteki windows.h, nazwy po angielsku, zamiast oceny=liczba+oceny; daj oceny+=liczba;, zapoznaj sie z camelCase, zamiast wynik=wynik+1; wynik++, czysty kod nie wymaga komentarzy,, nieuzywaj przestrzeni nazw

---

Refaktoryzacja nie oznacza "napisać coś jeszcze krócej". Oznacza napisać coś czytelniej _bez_zmiany_ działania pierwotnego programu. Używasz zmiennych globalnych (co jest błędem) i nie podzieliłeś programu na funkcje wykonujące określone prace.

---

polega na tym https://pl.wikipedia.org/wiki/Refaktoryzacja, wszytko co napisalem sprowadza sie do lepsze czytlnosci, odsmiecenia kodu

---

@QizmoPL: Komentuję post a nie Twoją wypowiedź :-) Jeśli będzie to Twoja wypowiedź, napiszę to wyraźnie :-) BTW, oczywiście masz rację co do uwag wystosowanych do @Huberti

---

komentarz :D komentarza to też refaktoryzacja?

---

Nie wiem jak to skomentować :-)

---

@mokrowski myslalem, ze to domnie

---

@mokrowski myślałem, że refaktoryzować oznacza poprostu zapisać instniejący już kod w taki sposób, aby jak najmniej obciążał komputer, ale działał wciąż tak samo.

Czy refaktoryzacja oznacza przepisać kod na czytelniejszy dla innych programistów?

@QizmoPL Dziękuję za skrócenie kodu i cenne rady :) Możesz powiedzieć za co odpowiada camelCase i dlaczego jest potrzebny do C++? 

 

Answer 1

Tu masz już zasadniczą część wykonaną. Co oznacza że jeszcze wiele można zrobić. Zgodnie jednak z zasadą refaktoryzacji (i wzorcami refaktoryzacji), nie została złamana konwencja działania jakakolwiek by była dobra czy zła.

Już nie ma sprzężeń przez zmienne globalne. Dalej przemyślał bym działanie poszczególnych funkcji. A już szczególnie nazwy zmiennych w liczenieSredniej(). Ja zachowałem oryginały ale w mojej ocenie nazwy są mylące.

Wyeliminował bym (bo można) także makra detekcji systemu operacyjnego. Przynajmniej co do opóźnień (teraz można to zrobić przez std::this_thread::sleep_for(...) )

Jak widzisz "wyszło dłuższe ale czytelniejsze" :-)

#include <iostream>
#include <cstdlib>
#include <ctime>
#include <cstdio>
#include <cstdlib>

#ifdef _WIN32
#include <conio.h>
#include <windows.h>
#elif __unix__
#include <unistd.h>
#endif

using namespace std;

void pokazInstrukcje();
int pobierzWybor();
void liczenieSredniej();
int pobierzLosowaLiczbe();
void zgadywanieLiczby();

int main() {
    int wybor;

    for(;;) {
        pokazInstrukcje();
        wybor = pobierzWybor();

        switch(wybor) {
            case 1:
                cout << "Wybrano program pierwszy\n\n";
                liczenieSredniej();
                break;

            case 2:
                cout << "Wybrano program drugi\n\n";
                zgadywanieLiczby();
                break;

            case 3:
                return EXIT_SUCCESS;
                break;

            default:
                break;
        }
    }

    return EXIT_SUCCESS;
}

void pokazInstrukcje() {
    cout << "Instrukcja:\n"
         "1. Kalkulator sredniej z ocen\n"
         "2. Gra w zgadnij jaka to liczba\n"
         "3. Wylacz program" << endl;
}

int pobierzWybor() {
    string napisWyboru;

    for(;;) {
        cout << "Podaj numer programu:" << endl;
        cin >> napisWyboru;

        if(napisWyboru == "1" || napisWyboru == "1.") {
            return 1;
        } else if(napisWyboru == "2" || napisWyboru == "2.") {
            return 2;
        } else if(napisWyboru == "3" || napisWyboru == "3.") {
            return 3;
        }

        cout << "Nieprawidlowy numer programu, sprobuj jeszcze raz za 3 sekundy"
             << endl;
        #ifdef _WIN32
        Sleep(3000);
        system("cls");
        #elif __unix__
        usleep(3000);
        system("clear");
        #endif
    }
}

void liczenieSredniej() {
    int przedmioty;
    int liczba;
    float oceny;
    int wynik = 1;
    int kalkulator = 0; //koniec resetu
    cout << "Kalkulator sredniej z ocen\n"
         "Podaj liczbe ocen do sredniej:";
    cin >> przedmioty;

    if((przedmioty > 0)) {
        while(kalkulator < przedmioty) {
            cout << "Wpisz ocene:";
            cin >> liczba;

            if(liczba >= 1 && liczba <= 6) {
                if(wynik == przedmioty) {
                    oceny = liczba + oceny;
                    cout << "Ilosc przedmiotow:\n" << przedmioty
                         << "\nSuma twoich ocen:\n" << oceny
                         << "\nTwoja srednia:\n" << oceny / przedmioty
                         << "\nPowodzenia w szkole :)" << endl;
                    ++kalkulator;
                    getchar();
                    getchar();
                    #ifdef _WIN32
                    system("cls");
                    #elif __unix__
                    system("clear");
                    #endif
                } else {
                    oceny += liczba;
                    ++wynik;
                    ++kalkulator;
                }
            } else {
                cout << "Wykryto nieprawidlowa liczbe, sprobuj jeszcze raz"
                     << endl;
                --kalkulator;
            }
        }
    } else {
        cout << "Liczba przedmiotow musi byc wieksza od 0!";
    }
}

int pobierzLosowaLiczbe() {
    cout << "Witaj! Pomyslalem sobie liczbe od 1 do 100" << endl;
    srand(time(NULL));
    return rand() % 100 + 1;
}

void zgadywanieLiczby() {
    int ile_prob = 0; //reset
    int liczba_losowa = pobierzLosowaLiczbe();
    int strzal;

    for(;;) {
        ++ile_prob;
        cout << "Sprobuj jaka to liczba:";
        cin >> strzal;

        if(strzal < liczba_losowa) {
            cout << "To za malo!\n";
        } else if(strzal > liczba_losowa) {
            cout << "To za duzo!\n";
        } else if(strzal == liczba_losowa) {
            cout << "Brawo, udalo Ci sie odgadnac! Wygrywasz w: "
                 << ile_prob << " probie!" << endl;
            getchar();
            getchar();
            #ifdef _WIN32
            system("cls");
            #elif __unix__
            system("clear");
            #endif
            break;
        }

        cout << "(to była twoja " << ile_prob << " proba)" << endl;
    }
}

 

Comments

Bardzo dziękuję, pewnie dużo się napracowałeś przy moim kodzie. Jednak większości z tego co napisałeś jeszcze nie rozumiem. Wciąż jestem we wczesnej fazie nauki tego języka. Jeszcze raz wielkie dzięki, zapiszę sobie twój kod i będę go na bieżąco analizował. :)