Ocena Pracy mojego Kalkulatora

Question

Witam, jestem w klasie 1 liceum i niedawno zacząłem programować w moim 1 języku czyli C++.

Obecnie jestem na 6 odcinku kursu na ''Pasja Informatyki'' więc wiem na razie niewiele :).

Będąc nie do końca usatysfakcjonowany kalkulatorem P. Zelenta, który przedstawił w jednym z odcinków postanowiłem stworzyć swój. Chciałbym usłyszeć wasze opinię na jego temat oraz ewentualne zmiany jakie mógłbym wprowadzić. Poniżej wklejam kod i z góry dziękuję za porady :).

#include <iostream>
#include <ostream>
#include <cmath>
#include <conio.h>

using namespace std;

char wybor;

int main()
{
double a, b, wynik;
int dzialanie;

for(;;)
{
cout<<"   MENU GLOWNE"<<endl;
cout<<"-----------------"<<endl;
cout<<"0. Wyjscie"<<endl;
cout<<"1. Dodawanie"<<endl;
cout<<"2. Odejmowanie"<<endl;
cout<<"3. Mnozenie"<<endl; 
cout<<"4. Dzielenie"<<endl;
cout<<"5. Pierwiastkowanie"<<endl;
cout<<"6. Potegowanie"<<endl; 
cout<<endl<<"Wybierz[0-6] ";

wybor=getch();

switch (wybor)
{
case '1': 
system("cls");
cout<<"======[Dodawanie]======"<<endl;
cout<<endl<<"Podaj 1 liczbe: ";
cin>>a;
cout<<"Podaj 2 liczbe: ";
cin>>b;
cout<<endl<<"     Suma= "<<a+b;
break;

case '2': 
system("cls");
cout<<"======[Odejmowanie]======"<<endl;
cout<<endl<<"Podaj 1 liczbe: ";
cin>>a;
cout<<"Podaj 2 liczbe: ";
cin>>b;
cout<<endl<<"    Roznica= "<<a-b;
break;

case '3': 
system("cls");
cout<<"======[Mnozenie]======"<<endl;
cout<<endl<<"Podaj 1 liczbe: ";
cin>>a;
cout<<"Podaj 2 liczbe: ";
cin>>b;
cout<<endl<<"    Iloczyn= "<<a*b;
break;

case '4': 
system("cls");
cout<<"======[Dzielenie]======"<<endl;
cout<<endl<<"Podaj 1 liczbe: ";
cin>>a;
cout<<"Podaj 2 liczbe: ";
cin>>b;
if(b==0) cout<<"Nie dzielimy przez 0!"<<endl;
else
cout<<endl<<"     Iloraz= "<<a/b;
break;

case '5':
system("cls");
cout<<"======[Pierwiastkowanie]======"<<endl;
cout<<endl<<"Podaj stopien pierwiastka: ";
cin>>b;
cout<<"Podaj liczbe do spierwiastkowania: ";
cin>>a;
if(b>0)	cout<<endl<<"          Wynik= "<<pow(a,(1/b));
else cout<<endl<<"Nie pierwiastkujemy przez 0!"<<endl;
break;

case '6': 
system("cls");
cout<<"======[Potegowanie]======"<<endl;
cout<<endl<<"Podaj liczbe ktora bedzie potegowana: ";
cin>>a;
cout<<"Podaj stopien potegi: ";
cin>>b;
cout<<endl<<"            Wynik= "<<pow(a,b)<<endl;
break;

default : cout<<endl<<"Koniec programu."<< endl;
exit(0);

}
getchar();getchar();
system("cls");
}
return 0;
}

 

Answer 1

Na pewno zacznij od formatowania kodu.

#include <iostream>
#include <ostream>
#include <cmath>
#include <conio.h>

using namespace std;

char wybor;

int main()
{
    double a, b, wynik;
    int dzialanie;

    for (;;)
    {
        cout << "   MENU GLOWNE" << endl;
        cout << "-----------------" << endl;
        cout << "0. Wyjscie" << endl;
        cout << "1. Dodawanie" << endl;
        cout << "2. Odejmowanie" << endl;
        cout << "3. Mnozenie" << endl;
        cout << "4. Dzielenie" << endl;
        cout << "5. Pierwiastkowanie" << endl;
        cout << "6. Potegowanie" << endl;
        cout << endl << "Wybierz[0-6] ";

        wybor = getch();

        switch (wybor)
        {
            case '1':
                system("cls");
                cout << "======[Dodawanie]======" << endl;
                cout << endl << "Podaj 1 liczbe: ";
                cin >> a;
                cout << "Podaj 2 liczbe: ";
                cin >> b;
                cout << endl << "     Suma= " << a + b;
                break;

            case '2':
                system("cls");
                cout << "======[Odejmowanie]======" << endl;
                cout << endl << "Podaj 1 liczbe: ";
                cin >> a;
                cout << "Podaj 2 liczbe: ";
                cin >> b;
                cout << endl << "    Roznica= " << a - b;
                break;

            case '3':
                system("cls");
                cout << "======[Mnozenie]======" << endl;
                cout << endl << "Podaj 1 liczbe: ";
                cin >> a;
                cout << "Podaj 2 liczbe: ";
                cin >> b;
                cout << endl << "    Iloczyn= " << a * b;
                break;

            case '4':
                system("cls");
                cout << "======[Dzielenie]======" << endl;
                cout << endl << "Podaj 1 liczbe: ";
                cin >> a;
                cout << "Podaj 2 liczbe: ";
                cin >> b;
                if (b == 0)
                    cout << "Nie dzielimy przez 0!" << endl;
                else
                    cout << endl << "     Iloraz= " << a / b;
                break;

            case '5':
                system("cls");
                cout << "======[Pierwiastkowanie]======" << endl;
                cout << endl << "Podaj stopien pierwiastka: ";
                cin >> b;
                cout << "Podaj liczbe do spierwiastkowania: ";
                cin >> a;
                if (b > 0)
                    cout << endl << "          Wynik= " << pow(a, (1 / b));
                else
                    cout << endl << "Nie pierwiastkujemy przez 0!" << endl;
                break;

            case '6':
                system("cls");
                cout << "======[Potegowanie]======" << endl;
                cout << endl << "Podaj liczbe ktora bedzie potegowana: ";
                cin >> a;
                cout << "Podaj stopien potegi: ";
                cin >> b;
                cout << endl << "            Wynik= " << pow(a, b) << endl;
                break;

            default:
                cout << endl << "Koniec programu." << endl;
                exit(0);
        }
        getchar();
        getchar();
        system("cls");
    }
    return 0;
}

Następnie usuń namespace, przynajmniej globalnie. 

Dlaczego? W przyszłości możesz korzystać z biblioteki, która zawiera dajmy na to funkcję pow. Przy globalnej przestrzeni nazw kompilator nie wie, która funkcja jest tą poprawną, bo obie noszą tą samą nazwę. Wiec po to jest prefiks std::pow i pow(z innej biblioteki). Nie wiem czy dość jasno, w razie wątpliwości na Stacku jest fajnie wytłumaczone. 

Zmienne globalne. Temat raczej dość... trzeba być ostrożnym. Raczej nie powinieneś ich używać bez dobrego uzasadnienia. Przenoś do zasięgu lokalnego main'na. 

Warto pozbyć się również nagłówków, które nie są przenośne. Co za tym idzie, usuwamy conio.h. Tutaj zaczynają się schody. Najprostsza odpowiedz: nie da się, ale możesz za to użyć dwóch możliwości, przynajmniej mi znanych. Ta łatwiejsza, użyć curses, albo ta mniej łatwa, użyć #ifdef _WIN32 i tak dalej. Akurat tego nie będę podawał, bo mi się nie chce, za to odsyłam tutaj https://stackoverflow.com/questions/1714245/difference-between-if-definedwin32-and-ifdefwin32

Masz zmienną, której nie używasz 

    int dzialanie;

Możesz usunąć, ale moim zdaniem lepiej będzie zamienić chara na inta. 

#include <iostream>
#include <ostream>
#include <cmath>
#include <conio.h>


int main()
{
    double a, b, wynik;
    int dzialanie;
    for (;;)
    {
        std::cout << "   MENU GLOWNE" << std::endl;
        std::cout << "-----------------" << std::endl;
        std::cout << "0. Wyjscie" << std::endl;
        std::cout << "1. Dodawanie" << std::endl;
        std::cout << "2. Odejmowanie" << std::endl;
        std::cout << "3. Mnozenie" << std::endl;
        std::cout << "4. Dzielenie" << std::endl;
        std::cout << "5. Pierwiastkowanie" << std::endl;
        std::cout << "6. Potegowanie" << std::endl;
        std::cout << std::endl << "Wybierz[0-6] ";

        std::cin>>wybor;

        switch (wybor)
        {
            case 1:
                system("cls");
                std::cout << "======[Dodawanie]======" << std::endl;
                std::cout << std::endl << "Podaj 1 liczbe: ";
                std::cin >> a;
                std::cout << "Podaj 2 liczbe: ";
                std::cin >> b;
                std::cout << std::endl << "     Suma= " << a + b;
                break;

            case 2:
                system("cls");
                std::cout << "======[Odejmowanie]======" << std::endl;
                std::cout << std::endl << "Podaj 1 liczbe: ";
                std::cin >> a;
                std::cout << "Podaj 2 liczbe: ";
                std::cin >> b;
                std::cout << std::endl << "    Roznica= " << a - b;
                break;

            case 3:
                system("cls");
                std::cout << "======[Mnozenie]======" << std::endl;
                std::cout << std::endl << "Podaj 1 liczbe: ";
                std::cin >> a;
                std::cout << "Podaj 2 liczbe: ";
                std::cin >> b;
                std::cout << std::endl << "    Iloczyn= " << a * b;
                break;

            case 4:
                system("cls");
                std::cout << "======[Dzielenie]======" << endl;
                std::cout << endl << "Podaj 1 liczbe: ";
                std::cin >> a;
                std::cout << "Podaj 2 liczbe: ";
                std::cin >> b;
                if (b == 0)
                    std::cout << "Nie dzielimy przez 0!" << std::endl;
                else
                    std::cout << std::endl << "     Iloraz= " << a / b;
                break;

            case 5:
                system("cls");
                std::cout << "======[Pierwiastkowanie]======" << std::endl;
                std::cout << std::endl << "Podaj stopien pierwiastka: ";
                std::cin >> b;
                std::cout << "Podaj liczbe do spierwiastkowania: ";
                std::cin >> a;
                if (b > 0)
                    std::cout << std::endl << "          Wynik= " << std::pow(a, (1 / b));
                else
                    std::cout << std::endl << "Nie pierwiastkujemy przez 0!" << std::endl;
                break;

            case 6:
                system("cls");
                std::cout << "======[Potegowanie]======" << std::endl;
                std::cout << std::endl << "Podaj liczbe ktora bedzie potegowana: ";
                std::cin >> a;
                std::cout << "Podaj stopien potegi: ";
                std::cin >> b;
                std::cout << std::endl << "            Wynik= " << std::pow(a, b) << std::endl;
                break;

            default:
                std::cout << std::endl << "Koniec programu." << std::endl;
                exit(0);
        }
        getchar();
        getchar();
        system("cls");
    }
    return 0;
}

Warto też będzie zamienić endla na \n, chyba, że potrafisz uzasadnić użycie endla. 

Dodatkowo używaj klamerek przy casach, porządkujesz kod i zawężasz zasięg zmiennych. 

exit(); jest dość dyskusyjne. 

Nazwy zmiennych a i b są bardzo nieodpowiednie. 

To co w casach powinno być w funkcjach. 

Tak z grubsza to wszystko. 

Comments

#include <iostream>
#include <ostream>
#include <cmath>
#include <conio.h>	


int main()
{
double a, b, wynik;
char choice;

for(;;)
{
	char choice;
std::cout<<"   MENU GLOWNE\n";
std::cout<<"-----------------\n";
std::cout<<"0. Wyjscie\n";
std::cout<<"1. Dodawanie\n";
std::cout<<"2. Odejmowanie\n";
std::cout<<"3. Mnozenie\n"; 
std::cout<<"4. Dzielenie\n";
std::cout<<"5. Pierwiastkowanie\n";
std::cout<<"6. Potegowanie\n"; 
std::cout<<"\nWybierz[0-6] ";

choice=getch();

switch (choice)
{
	case '0':
				exit(0);
		break;	

	case '1': 
			{
				system("cls");
				std::cout<<"======[Dodawanie]======\n";
				std::cout<<"\nPodaj 1 liczbe: ";
				std::cin>>a;
				std::cout<<"Podaj 2 liczbe: ";
				std::cin>>b;
				std::cout<<"\n     Suma= "<<a+b;
			}
		break;
	
	case '2': 
			{
				system("cls");
				std::cout<<"======[Odejmowanie]======\n";
				std::cout<<"\nPodaj 1 liczbe: \n";
				std::cin>>a;
				std::cout<<"Podaj 2 liczbe: ";
				std::cin>>b;
				std::cout<<"\n    Roznica= "<<a-b;
			}
		break;

	case '3': 
			{
				system("cls");
				std::cout<<"======[Mnozenie]======\n";
				std::cout<<"\nPodaj 1 liczbe: ";
				std::cin>>a;
				std::cout<<"Podaj 2 liczbe: ";
				std::cin>>b;
				std::cout<<"\n    Iloczyn= "<<a*b;
			}
		break;

	case '4': 
			{
				system("cls");
				std::cout<<"======[Dzielenie]======\n";
				std::cout<<"\nPodaj 1 liczbe: ";
				std::cin>>a;
				std::cout<<"Podaj 2 liczbe: ";
				std::cin>>b;
			if(b==0) 	std::cout<<"Nie dzielimy przez 0!\n";
			else		std::cout<<"\n     Iloraz= "<<a/b;
			}
		break;

	case '5':
			{
				system("cls");
				std::cout<<"======[Pierwiastkowanie]======\n";
				std::cout<<"\nPodaj stopien pierwiastka: ";
				std::cin>>b;
				std::cout<<"Podaj liczbe do spierwiastkowania: ";
				std::cin>>a;
				if(b>0)	std::cout<<"\n          Wynik= "<<pow(a,(1/b));
				else 	std::cout<<"\nNie pierwiastkujemy przez 0!\n";
			}
		break;

	case '6': 
			{
				system("cls");
				std::cout<<"======[Potegowanie]======\n";
				std::cout<<"\nPodaj liczbe ktora bedzie potegowana: ";
				std::cin>>a;
				std::cout<<"Podaj stopien potegi: ";
				std::cin>>b;
				std::cout<<"\n            Wynik= "<<pow(a,b);
			}
		break;

	default : std::cout<<"\nNie ma takiej opjci w MENU";

}
getchar();getchar();
system("cls");
}
return 0;
}

Trochę się pobawiłem i zmieniłem co nieco, teraz jest okej? 

Jutro jeszcze zmienię, że case'y będą w funkcjach :)

---

Nie mam pojęcia skąd ten pomysł na formatowanie. http://format.krzaq.cc/ 

Dlaczego choice to char?

No i zostaje kwestia getchar, system i exit.

---

Mam mały problem z tym clrscr();

Kiedy podmieniam go zamiast system("cls") wyskakuje mi bład

---

Jak nie powiesz jaki to błąd, trudno będzie mi pomóc.

---

[Error] 'clrscr' was not declared in this scope

---

clrscr jest niestandardową funkcją Borland Turbo C++ i nie występuje w innych kompilatorach.

---

A zamiast exit(0); i getchar(); co wstawić?

---

https://stackoverflow.com/questions/15945607/exit0-vs-return-0

 getchar() usuń

---

Tylko, że jak usunę getchar(); to wtedy np w tej sytuacji

std::cout<<"\nNie ma takiej opcji w MENU";	
			getchar();
			system("cls"); 

Napis "Nie ma takiej opcji w MENU" wcale się nie wyświetla.

---

Musisz wyczyścić bufor po getchar.