Ispravnost pisanja koda

Hej, imam pitanje.

Zanima me dali je ovo opcenito ispravan nacin programiranja i korištenja objekata?

Unaprijed hvala.

#include <iostream>
#include <stdlib.h>
#include <cmath>
#define PI 3.14
using namespace std;

class prizma
{
     public: void oplosje()
    {
       system("CLS");
       float oplosje_Prizme,B,P= 0;
       cout <<"Oplosje prizme"<<endl;
        cout <<"B:"; cin >> B;
       cout <<"P:"; cin >> P;
       oplosje_Prizme = 2*B+P;
       cout <<"O: " << oplosje_Prizme<<endl;
    }

       public: void volumen()
    {
       system("CLS");
       float volumen_Prizme,B,V= 0;
       cout <<"Volumen prizme"<<endl;
         cout <<"B:"; cin >> B;
       cout <<"V:"; cin >> V;
       volumen_Prizme = B*V;
       cout <<"V: " << volumen_Prizme<<endl;
    }
};

class valjak
{
          public: void oplosje()
    {
          system("CLS");
          float oplosje_Valjka,R,V= 0;
          cout <<"Oplosje valjka"<<endl;
          cout <<"R:"; cin >> R;
          cout <<"V:"; cin >> V;
          oplosje_Valjka = 2*R*PI*(R+V);
          cout <<"O: " << oplosje_Valjka<<endl;
     }

        public: void volumen()
     {
         system("CLS");
         float volumen_Valjka,R,V= 0;
          cout <<"Volumen valjka"<<endl;
         cout <<"R:"; cin >> R;
         cout <<"V:"; cin >> V;
         volumen_Valjka = pow(R,2)*PI*V;
         cout <<"O: " << volumen_Valjka<<endl;
      }
};

 int izbor()
       {
          int izbor,izbor1 = 0;

          cout <<"1. prizma"<<endl;
          cout <<"2. valjak" << endl;
          cout << endl;
          cout << "Unesi broj: "; cin >> izbor;
            cout << endl;
          switch(izbor)
          {
          case 1:
           cout <<"1. oplosje" << endl;
           cout << "2. volumen" << endl;
           cout << endl;
           cout << "Unesi broj: "; cin >> izbor1;
            if (izbor1 == 1){return 1;}
            else {return 2;}
            break;

          case 2:
            cout <<"1. oplosje" << endl;
            cout << "2. volumen" << endl;
            cout << endl;
            cout << "Unesi broj: "; cin >> izbor1;
            cout << endl;
            if (izbor1 == 1){    return 3;}
            else {return 4;}
            break;
    }
}


int main()
{
    valjak _valjak;
    prizma _prizma;
    switch(izbor())
    {
    case 1:
    _prizma.oplosje();
    break;
   
    case 2:
    _prizma.volumen();
    break;

    case 3:
    _valjak.oplosje();
    break;

    case 4:
    _valjak.volumen();
    break;

    default:
    exit(1);
    }

    system("PAUSE");
    return 0;
}

Samo bih još dodao da je u ovom slučaju najbolje napraviti interface Tijelo s dvije metode bez parametra koje račnuaju i vraćaju vrijednost oplošja i volumena, pa onda za svako tijelo kreiraš klasu koja implementira taj interface. Ako trebaš baratati s oplošjem i volumenom dalje u programu, na taj način možeš pristupiti oplošju i volumenu svakog tijela, bez da vodiš računa o kojem se konkretno tijelu radi.

Trenutno nemam preterano vremena, a u zadnje vrijeme sam u C# i Javi, pa bih mogao nešto i pogriješiti. Mislim da je ovo ok za početak:

http://accu.org/index.php/journals/233

E, desilo se, a što mogu, moram ići oćale tražiti

Ispravljeno!

Koristiš C++, imaš jednu statički alociranu strukturu i imaš funkcije koje koriste tu jednu statički alociranu strukturu. To je besmisleno. Isto tako si sve te funkcije mogao ugurati u tu strukturu pa ne bi onda svuda morao pisati rover.blablabla.

Također, zašto komplicirati s nazivima varijabli? Ako imaš strukturu Rover, zašto ti sve varijable počinju s rover? Ako se zna, pošto su u toj strukturu, da se odnose upravo na rover? Također, zašto si nedosljedan? U prve dvije varijable nemaš podvlake (_), u ostalima imaš.

Umjesto prva četiri if-a si mogao koristiti switch. Ili, još bolje, mogao si imati jedno polje s vrijednostima 'N', "E', 'S', 'W' pa jednostavno dobiti te vrijednosti preko indeksa (a indeks ti je roverHeading_numeric-1).

Što ako korisnik upiše malo slovo?

Kako korisnik zna što treba upisati za H?

Zar ne bi za instrukciju 'M' bilo smislenije da imaš jedan vanjski uvjet (ako je == 'M') pa da onda u unutrašnjem bloku provjeravaš heading_numeric? Znači:

if (rover.rover_Instruction[i] == 'M') {

  switch(rover.roverHeading_numeric) {

   case 1: rover.roverPositionY ++; break;

   case 2: rover.roverPositionX ++; break;

    ...

  }

}

Ovo ti nema smisla. Unos parametara je metoda specifična za svako tijelo te bi se zato tamo trebala i nalaziti. S obzirom da s tijelom nemaš  što raditi bez parametara i da nemaš zašto kreirati tijelo dok ne znaš parametre, nema razloga izmišljati metodu Unos parametara kada to može raditi konstruktor. U GeometrijskoTijelo ima smisla staviti konstantu PI jer je koristimo za više tijela i za sve je ista (iako je i to upitno jer je u nekima tijelima nepotrebna i samo zauzima memoriju, ovo je više radi preglednosti koda), te po istoj logici se mogu ubaciti i metode za računanje površina pravokutnika, trokuta i kruga koje bi poslje mogli koristiti za računanje oplošja i volumena (morale bi biti protected).

class GeometrijskoTijelo
{
  protected:
      double PI = 3.14159265;

  public:
      virtual double Oplosje() = 0;
      virtual double Volumen() = 0;
};


class Valjak : public GeometrijskoTijelo
{
  private:
      double _polumjer, _visina;

  public:
      Valjak(double polumjer, double visina)
      {
          _polumjer = polumjer;
          _visina = visina;
      }
      double Oplosje()
      {
          return 2 * _polumjer * PI * (_polumjer + _visina);
      }
      double Volumen()
      {
          return _polumjer * _polumjer * PI * _visina;
      }
};


class Prizma : public GeometrijskoTijelo
{
  private:
      double _visina, _baza, _pobocje;

  public:
      void Prizma(double baza, double pobocje, double visina)
      {
          _baza = baza;
          _pobocje = pobocje;
          _visina = visina;
      }
      double Oplosje()
      {
          return 2 * _baza + _pobocje;
      }
      double Volumen()
      {
          return _baza * _visina ;
      }
};


int main()
{
    GeometrijskoTijelo* valjak = new Valjak(5.4, 8.5);
    GeometrijskoTijelo* prizma = new Prizma(3.4, 5.3, 4.7);

    cout<< "Oplosje valjka: " << valjak->Oplosje() << endl;
    cout<< "Volumen valjka: " << valjak->Volumen() << endl;
    cout<< "Oplosje prizme: " << prizma->Oplosje() << endl;
    cout<< "Volumen prizme: " << prizma->Volumen() << endl;
    delete valjak;
    delete prizma;
    return 0;
}

Ovo ti nema smisla. Unos parametara je metoda specifična za svako tijelo te bi se zato tamo trebala i nalaziti. S obzirom da s tijelom nemaš što raditi bez parametar . . .

Jest, bolje je, sa jednom primjedbom, compiler mi ne dozvoljava inicijalizaciju podatkovnog člana ako ga ne označim kao konstantnog,

a onda opet prihvaća samo integer tip, zato sam stavio u define

Aha, da, to sam zaboravio ...

Znaci trebalo bi jos dodati delete pravokutnik ako se ne varam !?

Imam jos jedno pitanje, zasto kod ovog:

virtual double Povrsina() = 0;

mora ići na kraju = 0, ako toga nema onda ne radi ?

Znači kad koristimo virtualne funkcije, preko njih ustvari pozivamo prave funkcije koje zapravo nešto rade a u svakoj klasi ta funkcija s istim imenom moze raditi nešto drugo ?

Napravio si kardinalnu pogrešku! Pravokutnik nije geometrijsko tijelo nego geometrijski lik.

Provrti malo po forum tražilici...imaš na ovom forumu više detaljnih objašnjenja o klasama ,objektima i virtualnim funkcijama.