Prośba o ocenę kodu

Question

Cześć, napisałem taki programik i chciałbym prosić, żebyście go ocenili, wskazali ewentualne błędy, rzeczy do poprawy itd.

#include <iostream>
#include <iomanip>
#include <cstdlib>

using namespace std;

int main()
{
    bool powrot(false);
    menu:

    do
    {

        cout << "|---------------------------WITAJ------------------------------|"<<endl;
        cout <<"|-----------------WYBIERZ OPCJE Z MENU PONIZEJ ----------------|"<<endl;
        cout <<"|--------------------------------------------------------------|"<<endl;
        cout << "\n";

        cout << "MENU GLOWNE \n";
        cout << "----------- \n";
        cout << "1.Obliczanie sumy \n";
        cout << "2.Wzor \n";
        cout << "3.Tabliczka mnozenia \n";
        cout << "4.x i y \n";
        cout << "\n";
        cout << "Wybieram: ";

        int menu;

        while(!(cin >> menu)||(!(menu>=1 && menu<=4)))
        {
            cin.clear();
            cin.ignore();
            cout << "Podana wartosc nie spelnia warunkow wyboru.\n";
            cout << "Wybierz opcje: ";
        }

        switch(menu)
        {
        case 1:
        {
            cout << "\nWybrano Obliczanie sumy \n";
            cout << "Podaj N: ";
            int N;

            while(!(cin >> N)||(!(N>=1)))
            {
                cin.clear();
                cin.ignore();
                cout << "Podana wartosc nie spelnia warunkow! Podaj N: ";
            }

            double wynik{0};
            double wzor;

            for(int i=1; i<=N; ++i)
            {
                wzor =((i+1.0)/(2.0*i*i+1.0));
                wynik+=wzor;
            }
            cout << "Suma wynosi: " << wynik;

            break;
        }

        case 2:
        {
            cout << "\n";
            cout << "\nWybrano wzor \n";
            cout << "Podaj LW(liczbe wierszy): ";
            int LW;

            while(!(cin >> LW)||(!(LW>0)))
            {
                cin.clear();
                cin.ignore();
                cout << "Podana wartosc nie spelnia warunkow. Podaj LW: ";
            }

            for(int i=1; i<=LW; i++)
            {
                for(int j=1; j<=i; j++)
                {
                    cout << "*";
                }
                cout << "\n";
            }

            break;
        }

        case 3:
        {
            cout << "\nWybrano tabliczke mnozenia \n";
            cout << "Podaj N1(maksymalnie 10): ";
            unsigned int N1;

            while(!(cin >> N1)|| (!(N1<=10)))
            {
                cin.clear();
                cin.ignore();
                cout << "Nie spelniono warunkow. Podaj N1: ";
            }

            for(int z=1; z<=N1; ++z)
            {

                cout << setw(4) << z;
            }

            for(int i=1; i<=N1; ++i)
            {
                cout << "\n" << i;

                for(int j=1; j<=N1; ++j)
                {
                    cout << setw(4) << i*j;
                }

            }

            break;
        }

        case 4:
        {
            cout << "\nWybrano x i y \n";
            cout << "Podaj x: ";
            unsigned int x;
            cin >> x;
            cout << "Podaj y: ";
            unsigned int y;
            cin >> y;
            int w=1;

            if(y==0)
            {
                cout << 1;
                return 0;
            }

            for(int i=1; i<=y; i++)
            {
                w*=x;
            }
            cout << w;

            break;
        }

        }
        cout << "\n" << "\n";
    }
    while(powrot);

    cout << "Czy chcesz powtorzyc? (0 - nie, 1 - tak): ";
    int decyzja;
    cin >> decyzja;
    if(decyzja==0)
    {
        return 0;
    }
    else if(decyzja==1)
    {
        system("cls");
        goto menu;
    }

    cin.ignore();
    cin.get();
    return 0;
}

 

Answer 1

1. Sam kod jest nawet oksy, ale jest kilka ale

2. Goto, to zlo, moze nie w tym programie, bo jest tylko jedno i widac dokad skaczesz, ale ale wyobraz sobie program/funkcje na 3k linii koeu i z 50 jumpami ;), po to sa petle i warunki, zeby to ominac

3. Modularyzacja, czyli male funkcje, z czego kazda powinna spelniac tylko jedno zadanie. Np, funkcja sumujaca itd.

4. Style code nie jest konsewentny, tzn, skoro robisz:

cout << " hrjebej ";

To zrob to samo wszedzie:

if (zmienna == wartosc) {
} 

I tak dalej ;) nsjlepiej jest chyba poczytac o jakos juz znanych style code, typu chrome i sie to trzymac.

5. Przestrzen nazw, tu nie jest grozna, ale golnie odradza sie ich uzywa. Tzn zamiast

using namespace std;
cout


Robic

std::cout

6. Trzymaj sie angielskich nazw zmiennych, dzieki temu na forach obcojezycznych, rowniez uzyskasz szybka pomoc, w innym przypadku trzeba poswiecic wiecej czasu, zeby sie wczytac w kod

7. Odnosnie 7, zmienne powinny miec dusze, a nie tylko nazwe ;). Zmienna x, y, z nie mowia nic, za to zmienna sum lub numberOf... wyrazaja wiecej niz jeden znak

8. Unsigned int, moze byc lekko niebezpieczny, ale ale od sie to ogolnie do integer under/overflow, ktory jest UB i program sie pewnie sypnie, dlstego wsrto zabezpieczac w tych i innych miejscach program. Pieknych przykladem jest case numer 4. Gdzie wykonujesz multiplikacje liniowa, wiec dla duzego y, latwo przekroczyc wsrtosc inta.

9. iomanip, nie widze zadnej funkcji z tej libki, chociaz tez nie znam wszytskich. Moze byc tak, ze jest bez sensu podpieta

Comments

Dziękuję za wszelkie sugestie i uwagi. Dopiero się uczę programowania w C++, dlatego takie byki są xd Goto dałem bo nie za bardzo miałem pomysł na powrót do menu. Tutaj miałem zrobić program który będzie się wykonywać tylko w głównej funkcji main(). Mam trochę bałaganu w tym nazewnictwie i konsekwencji stylu, a to dlatego, że użyłem tutaj CodeBlocks-owej opcji "Format use AStyle" xd. Teraz już postaram się pisać z angielskimi nazwami. Zawsze jakoś staram się zabezpieczać poszczególne kawałki kodu - widać to u mnie po tych pętlach

while()

sprawdzających każdą wpisaną wartość przed jej użyciem. Kiedyś zobaczyłem to sprawdzanie tą pętlą i jakoś tak tego używam gdzie mogę xd. Bez biblioteki 

<iomanip> 

nie wykona się

setw() 

:(.

Jeszcze raz wielkie dzięki za te uwagi! 

---

Oh, nie zauwazylem setw. Btw. CodeBlocks bym zmienil i zaczal uzywac albo VisualStudio, CLion lub jakis edytor + cmake + kompilator.

Nie rozumiem po co wracac do memu, jezeli ktos sam z niego wyszedl, to bym zakonczyl akcje. Ale jak mowil poczytac o funkcjsch i odrazu problem z goto zniknie

---

Okej. Znaczy miałem program zrobić tak, żeby po wykonaniu się, któregoś z wybranych z menu opcji, pytał się na koniec czy chcesz wykonać coś jeszcze czy już wyjść.