Kontrola kódu OOP C++ – C / C++ – Fórum – Programujte.com

Zdravím, 

jako začátečník hledám někoho, kdo by byl ochotný projet můj kód a pokusil by se vyhledat případné chyby (překladač mi žádné nehlásí, nicméně program nefunguje jak by měl). Jedná se o krátký a intuitivní kód, který zašlu e-mailem.

a kolik za tu kontrolu intuitivního kodu nabízíš peněz :)

#2 Jerry
Ahoj Jerry,

předpokládám, že si dotyčný cenu určí na základě vlastní analýzy :-)

A proc si to nedal rovnou sem, by to bylo rychlejsi a zadara

Dát to sem klidně můžu, otázkou je, kdo to zadarmo bude chtít zkontrolovat :-)

Požadovaná funkcionalita:

- přidání nové rostliny na záhon
- odebrání vybrané rostliny ze záhonu
- výpis informací o vybrané rostlině včetně její ceny
- výpočet celkové ceny všech rostlin na záhoně
- výpis informací a výpočet celkové ceny všech rostlin konkrétního druhu

Záhon má předem definovanou kapacitu (počet rostlin), takže nelze přidávat
do nekonečna, rovněž nelze odebírat ze záhonu, který nemá žádné rostliny.   
  
Rostlina:
- druh (např. růže, čimišník, denivka ...)
- barva květu
- výška

Každá rostlina má určitou cenu (v Kč), která se vypočítá z výše uvedených 
informací.

Program bude uchovávat stav záhonu v souboru, kam se budou promítat
veškeré změny (přidání, odebrání).

Mé řešení:

 main.cpp

#include <iostream> 
#include <fstream>
#include <string>
#include <sstream>
#include <vector>
#include <Rostlina.h>
#include <Zahon.h>
using namespace std;


void vypisMenu(){
    cout << "+++++++++++++++++++++++++++++++++++++++++++++++++++++++++" << endl;
    cout << "+                                                       +" << endl;
    cout << "+          1) Pridat rostlinu                           +" << endl;
    cout << "+          2) Odebrat rostlinu                          +" << endl;
    cout << "+          3) Vypsat informace o rostline               +" << endl;
    cout << "+          4) Vypocitat celkovou cenu                   +" << endl;
    cout << "+          5) Vypsat informace o konketnim druhu        +" << endl;
    cout << "+                                                       +" << endl;  
    cout << "+++++++++++++++++++++++++++++++++++++++++++++++++++++++++" << endl;
    }
    
void vypis(Rostlina rostlina);
void vypisCenuDruhu(Rostlina);
void odebratRostlinu(Zahon, int);
void pridatRostlinu(Zahon);
void cenaVsechRostlin(Zahon);
void cenaVsechDruhu(Zahon);




int main(int argc, char **argv){
    
    vypisMenu();
    
    int vstup = 0;
    int cena = 0;
    int i = 1;
    const int limit = 50;
    
    
    cout << " Zadejte vasi volbu: " << endl;
    cin >> vstup;
  
    Zahon zahon("Soubor.txt");
   for(Rostlina rostlina : zahon.rostliny){
        cout << i << ". ";
        vypis(rostlina);
        cena = cena + rostlina.vypisCenu();
        i++;
    }
    cout << "-----------------------------------------------" << endl;
    
    switch(vstup){
        
        case 1: {
            if(zahon.rostliny.size() > limit){
                cout << "Na tento zahon pridat vice rostlin !" << endl;
                break;
            }
            else{
                pridatRostlinu(zahon);
                
            }
        }
        
        break;
        
        case 2: {
            cout << "Zadejte prvek ktery chcete odebrat: ";
            int prvek = 0;
            cin >>prvek;
            
            if(prvek > 0 && prvek < limit && prvek < zahon.rostliny.size()){
                odebratRostlinu(zahon, prvek);
            }
            else{
                cout << "Nepaltny vstup !";
                return 0;
            }
            break;
        }
        
       case 3: { 
            cout << "Zadejte rostlinu, kterou chcete vypsat: ";
            int prvek = 0;
            cin >> prvek;
            if(prvek > 0 && prvek < limit && limit > zahon.rostliny.size()){
              vypis(zahon.rostliny.at(prvek-1));
            }
            else{
                cout << "Neplatny vstup !";
                return 0;
            }
            break;
        }
        
        case 4: { 
            cenaVsechRostlin(zahon);
            break;
        }
        
        case 5: {
            cenaVsechDruhu(zahon);
            break;
        }
        
        default:{ 
            cout << "Neplatna volba! " << endl; 
            return 0;
        }
    }
    ofstream soubor;
    soubor.open("soubor.txt");
    for(Rostlina rostlina : zahon.rostliny){
        soubor << rostlina.vypisDruh() << " " << rostlina.vypisBarvuKvetu() << " " << rostlina.vypisVysku() << endl;
    }
}   

void vypis(Rostlina rostlina){
    cout << "Druh: " << rostlina.vypisDruh() << endl;
    cout << "Barva kvetu: " << rostlina.vypisBarvuKvetu() << endl;
    cout << "Vyska: " << rostlina.vypisVysku() << endl;
    cout << "Cena: " << rostlina.vypisCenu() <<endl;
}  

void pridatRostlinu(Zahon zahon){
    cout << "Zadejte druh rostliny: " << endl;
    string vstup_druh;
    cin >> vstup_druh;
            
    cout << "Zadejte barvu kvetu: " << endl;
    string vstup_barva;
    cin >> vstup_barva;
            
    cout << "Zadejte vysku rostliny: " << endl;
    int vstup_vyska;
    cin >> vstup_vyska;
            
    stringstream ss;
    ss << vstup_druh << " " << vstup_barva << " " << vstup_vyska << endl;
    string str;
    str = ss.str();
            
    Rostlina novaRostlina(str);
    zahon.rostliny.push_back(novaRostlina);
    int i = 1;
    for(Rostlina rostlina : zahon.rostliny){
        cout << i << ". ";
        vypis(rostlina);
        i++;
    }
}

void vypisCenaDruhu(Rostlina rostlina){
    cout << "Cena druhu je: " << rostlina.vypisCenuDruhu() << endl;
}

void odebratRostlinu(Zahon zahon, int prvek){
    int i = 1;
    zahon.rostliny.erase(zahon.rostliny.begin()+prvek-1);
    for(Rostlina rostlina : zahon.rostliny){
        cout << i << ". ";
        vypis(rostlina);
        i++;
    }
}

void cenaVsechRostlin(Zahon zahon){
    int cena = 0;
    for(Rostlina rostlina : zahon.rostliny){
        cena = cena + rostlina.vypisCenu();
    }
    cout << "Cena vsech rostlin je: " << cena << endl;
}

void cenaVsechDruhu(Zahon zahon){
    int cenaDruhu;
    for(Rostlina rostlina : zahon.rostliny){
        cenaDruhu = cenaDruhu + rostlina.vypisCenuDruhu();
    }
    cout << "Cena vsech druhu rostlinje: " << cenaDruhu << endl;
}

Rostlina.cpp

#include "Rostlina.h"
#include <iostream>
#include <cstring>
#include <sstream>
using namespace std;

Rostlina::Rostlina(string infoRostlina){
    
    stringstream rostlinaStream(infoRostlina);
    rostlinaStream >> druh;
    rostlinaStream >> barvaKvetu;
    rostlinaStream >> vyska;
}

string Rostlina::vypisDruh(){
    return druh;
  }

string Rostlina::vypisBarvuKvetu(){
    return barvaKvetu;
  }   
  
int Rostlina::vypisVysku(){
    return vyska;
  } 
  
int Rostlina::vypisCenu(){
    int cena = 0;
    
    if(druh == "ruze"){
        cena = cena + 30;
        } else if (druh == "cimisnik"){
            cena = cena + 25;
            }  else if (druh == "denivka"){
                cena = cena + 20;
                } else if (druh == "levandule"){
                    cena = cena + 15;
                    } else if (druh == "tulipan"){
                        cena = cena + 10;
                        } else { cout << "Tuto rostlinu bohuzel nemame."  << endl;
                                 } 
   
     if(barvaKvetu == "cervena"){
         cena = cena + 15; 
        } else if (barvaKvetu == "fialova"){
            cena = cena + 14;
            }  else if (barvaKvetu == "bila"){
                cena = cena + 13;
                } else if (barvaKvetu == "ruzova"){
                    cena = cena + 12;
                    } else if (barvaKvetu == "zelena"){
                        cena = cena + 11;
                        } else { cout << "Rostliny o takove barve kvetu bohuzel nemame."  << endl;             
    
    if (vyska > 0 && vyska < 15){
        cena = cena + 5;
        } else if (vyska > 15 && vyska < 30){
            cena = cena + 10;
            } else if (vyska > 30 && vyska < 60 ){
            cena = cena + 40;
               } else { cout << "Zadana vyska je neplatna. Pouzijte prosim vysku od 0 do 60 cm" << endl;
                   }

     return cena;
  } 
   }
  
  int Rostlina::vypisCenuDruhu(){
      int cena = 0;
      
       if(druh == "ruze"){
        cena = cena + 30;
        } else if (druh == "cimisnik"){
            cena = cena + 25;
            }  else if (druh == "denivka"){
                cena = cena + 20;
                } else if (druh == "levandule"){
                    cena = cena + 15;
                    } else if (druh == "tulipan"){
                        cena = cena + 10;
                        } else { cout << "Tuto rostlinu bohuzel nemame."  << endl;
                                 }
         return cena;
      }

Rostlina.h

#ifndef __Rostlina_h_
#define __Rostlina_h_
#include <iostream> 
#include <fstream>
#include <string>
#include <vector>
using namespace std;

class Rostlina
{
private:

  string druh;
  string barvaKvetu;
  int vyska;
  
public: 

    Rostlina(string infoRostlina);
    string vypisDruh();
    string vypisBarvuKvetu();
    int vypisVysku();
    int vypisCenu();
    int vypisCenuDruhu();
  
    
    

};

#endif // __Rostlina_h_

Zahon.cpp

#include "Zahon.h"
#include <iostream> 
#include <fstream>
#include <sstream>
#include <string>
#include <vector>
using namespace std;


Zahon::Zahon(string fin){

    ifstream rostlinaSoubor(fin.c_str());
    string radek;
    while(!rostlinaSoubor.eof()){
        
        getline(rostlinaSoubor, radek);
        
        Rostlina rostlina(radek);
        rostliny.push_back(rostlina);
    }
    rostlinaSoubor.close();
}

Zahon.h

#ifndef __Zahon_h_
#define __Zahon_h_
#include "Rostlina.h"
#include <iostream> 
#include <fstream>
#include <string>
#include <vector>
using namespace std;

class Zahon
{
public:
    Zahon(string fin);
    vypisRostliny();
    vector<Rostlina> rostliny;
    
};

#endif // __Zahon_h_

#5 Yenkee
no a jak se to chova? Na rozdil od toho co to delat ma?

spustí se menu, požádá uživatele o vstup a po zadání vstupu nereaguje :-)

#7 Yenkee
Problemu je tam vic

Ten hlavni, proc ti to nefunguje, je:  while (!rostlinaSoubor.eof()) { ... krapet to nefacha pri pokusu otevrit neexistujici soubor (coz netestujes). Takze bud tam dej proste  while (rostlinaSoubor) { ..., nebo musis overit, jestli se otevrel soubor.

Dale napriklad vypisCenu neco vrati, az kdyz zadas nespravnou barvu. Jinak to nema co vracet.

Nesmyslny nazvy metod jako: vypisCenu, vypisDruhu  - nic to nevypisuje (krom chybovych hlasek), tak je pitomost nazyvat to vypisCokoliv.

Taktez using namespace std; je v hlavickovych souborech povazovano za tezky zlozvyk. Kdyz uz, tak jen v cpp souborech a i tak je lepsi bud si vytahat konkretni tridy a tak:  using std::string; using std::sstream;  Neni nic horsiho, nez pracovat s nejakou knihovnou, ktera zasere namespace miliardou symbolu, a clovek pak nikdy netusi, co se nakonec pouzije....

Dale mas zbytecne includy v headerech. Staci jen to co tam opravdu pouzivas, zbytek muze byt az v .cpp

Taktez udrzovat si vsechny ty barvy a typy a tak v retezcich je zbytecne, nejakej vhodnej prevod by te usetril tech if - else if --- ke si nakonec narval i tu chybu se spatnou zavorkou (kterou nevidis, protoze odsazujes blbe)

(to je zatim to, na co sem narazil)

A nauc se pouzivat debugger, hned bys zjistil tu hlavni chybu

Mimochodem, intuitivni bych to taky moc nenazyval :D

Mnohokrát děkuji, jdu na to mrknout :-) 

Edit: Nabízím finanční odměnu za případnou korekturu, už si nevím rady

        kontakt: yenkee.gaming@seznam.cz

#9 Yenkee
uz ses aspon dostal pres tu nekonecnou smycku?

#10 KIIV
No tak to si nejsem jistý, ale když jsem z hlavičkových souborů odebral namespace std, tak to tu mám samou chybu

#11 Yenkee
Musis pak hazet  std::string, std::vector  (tusim, ze nic jineho tam ani neni)

Je to jen zlozvyk, u domacich ukolu se to asi da prezit. Ale na druhou stranu bys aspon mohl vedet, k cemu to vlastne je :)

Pokud ten program má být napsán objektově, měl by ses zbavit těch switchů a špaget z ifů. Při požadavku přídání dalšího druhu rostliny by se hotové třídy již neměly modifikovat, ale pouze se přidá další třída, která bude potomkem třídy Rostlina.

Stále platí nabídka odměny pro toho, kdo by byl ochoten opravit program do funkční podoby

#14 Yenkee
copak ? on ti nikdo neporadil zadara ??? :))) to je divný ne ?

#15 Jerry

Ale ano, odhalili mi spoustu chyb, které se snažím postupně vyřešit. Například už se mi podařilo opravit "vypisNeco" metody, aby opravdu vracely to, co potřebuji. Nabídku na opravu jsem zadal proto, abych ušetřil investovaný čas, člověku, který tomu rozumí to zabere 1/10 času. Já se programovat učím 2 měsíce, takže mi bohužel chybí ten potřebný nadhled a u každého, byť sebemenšího problému, se zaseknu na dlouhou dobu.

Tvůj komentář byl více než zbytečný.

Je tvůj cíl naučit se programovat? Pak hotové řešení tě moc nenaučí. 

hu

#17 hlucheucho

Mým cílem je projít povinným kurzem programovaní (bohužel i ve specializaci, která programování nevyžaduje je jeden povinný), nikoli se naučit programovat. Navíc teoretickou část mám již za sebou, už jen dodělat program.

#16 Yenkee
mimochodem u tech vypisNeco, jsem narazel zrovna na nazev te metody zacinajici "vypis", kdyz to nic nevypisuje.

#16 Yenkee
Zveřejnil jsi to, co už máš, i když to zatím nefunguje. To je dobrý začátek. Domácí úkoly tu sice neděláme, ale můžeme kolem tvého kódu diskutovat, což ti může přinést mnohem víc.

V C++ nedělám, od toho jsou tu povolanější.

#19 KIIV
Také se mi ty názvy metod nelíbí. Jak správně uvádíš, dělají něco jiného. Osobně dávám přednost jednoslovním názvům - slovesům.

zatím jsem zkusil opravit pouze třídu rostlina, už by to mělo vracet to, co je potřeba

rostlina.h

#ifndef __Rostlina_h_
#define __Rostlina_h_

class Rostlina
{
private:

  std::string druh;
  std::string barvaKvetu;
  int vyska;
  
public: 

    Rostlina(std::string infoRostlina);
    std::string vypisDruh();
    std::string vypisBarvuKvetu();
    int vypisVysku();
    int vypisCenu();
    int vypisCenuDruhu();
  
    
    

};

#endif // __Rostlina_h_

rostlina.cpp

#include <iostream>
#include <sstream>
#include <stdlib.h>
#include "Rostlina.h"
using namespace std;


Rostlina::Rostlina(std::string infoRostlina){
    
    stringstream rostlinaStream(infoRostlina);
    rostlinaStream >> druh;
    rostlinaStream >> barvaKvetu;
    rostlinaStream >> vyska;
}

std::string Rostlina::vypisDruh(){
    cout << "Druh: " << druh << endl;
  }

std::string Rostlina::vypisBarvuKvetu(){
    cout << "Barva kvetu: " << barvaKvetu << endl;
  }   
  
int Rostlina::vypisVysku(){
    cout << "Vyska: " << vyska << endl;
  } 
  
int Rostlina::vypisCenu(){
    int cena = 0;
    
     if(druh == "ruze")
        {
        cena = cena + 30;
        } else if (druh == "cimisnik")
            {
            cena = cena + 25;
            }  else if (druh == "denivka")
                {
                cena = cena + 20;
                } else if (druh == "levandule")
                    {
                    cena = cena + 15;
                    } else if (druh == "tulipan")
                        {
                        cena = cena + 10;
                        } else { cout << "Tuto rostlinu bohuzel nemame."  << endl;
                                 exit(EXIT_FAILURE);
                                 } 
   
     if(barvaKvetu == "cervena")
         {
         cena = cena + 15; 
         } else if (barvaKvetu == "fialova")
            {
            cena = cena + 14;
            }  else if (barvaKvetu == "bila")
                {
                cena = cena + 13;
                } else if (barvaKvetu == "ruzova")
                    {
                    cena = cena + 12;
                    } else if (barvaKvetu == "zelena")
                        {
                        cena = cena + 11;
                        } else { cout << "Rostliny o takove barve kvetu bohuzel nemame."  << endl; 
                                 exit (EXIT_FAILURE);            
                                }
    if (vyska > 0 && vyska < 15)
        {
        cena = cena + 5;
        } else if (vyska > 15 && vyska < 30)
            {
            cena = cena + 10;
            } else if (vyska > 30 && vyska < 60 )
            {
            cena = cena + 40;
            } else { cout << "Zadana vyska je neplatna. Pouzijte prosim vysku od 0 do 60 cm" << endl;
                    exit (EXIT_FAILURE);
                   }

     
     cout << "Cena této rostline je: "  << cena << "Kč." << endl;
  }     
  
  int Rostlina::vypisCenuDruhu(){
      int cena = 0;
      
       if(druh == "ruze"){
        cena = cena + 30;
        } else if (druh == "cimisnik"){
            cena = cena + 25;
            }  else if (druh == "denivka"){
                cena = cena + 20;
                } else if (druh == "levandule"){
                    cena = cena + 15;
                    } else if (druh == "tulipan"){
                        cena = cena + 10;
                        } else { cout << "Tuto rostlinu bohuzel nemame."  << endl;
                                 exit (EXIT_FAILURE);
                                 }
         cout << "Cena " << druh <<" je: " << cena << " Kč" << endl;
      }

#22 Yenkee
Snaž se dostat cin a cout ven z metod. Vstupy a výstupy mají být mimo objekty nebo se předávají přes parametry. Nebudeš pak mít duplicity v metodách vypisCenu() a vypisCenuDruhu(). Místo toho ti metody cenaDruhu(), cenaBarvy() a cenaVyska() budou vracet čísla, která si už sečteš a zformátuješ dle potřeby. Případně metoda cena() zavolá tyto tři metody a jejich výsledky sečte. Tím také zcela odpadne interní proměnná cena.

Nechej svůj editor ten kód zformátovat. Stačilo mi, abych ho prohnal programem indent a rázem se to dá číst.

Je dobrým zvykem soubor pojmenovávat podle názvu třídy. Třída Rostlina by tedy měla být v souboru Rostlina.cpp. Respektuj velikost písmen.

Zdravím, většinu funkcí se mi již podařilo opravit, nyní jsem se zasekl na funkci, která by dokázala odebrat vybranou rostlinu, kterou přidávám následující funkcí. 

void pridejRostlinu(rostlina** rostliny, int& pocet)
{

    //zde vstupy od uživatele a podmínky, které je šetří -pro přehlednost 
    // jsem je vynechal

    rostlina* rostliny2 = new rostlina[pocet + 1];
    for (int i = 0; i < pocet; i++)
    {

        rostliny2[i] = (*rostliny)[i];
    }
    rostliny2[pocet] = rostlina(druh, barvaKvetu, vyska);
    *rostliny = rostliny2;
    pocet++;
}

#24 Yenkee
Můžeš zase vytvořit nové pole, ale při kopírování vynecháš prvek, který chceš odstranit.

Pokud jsi na tohle řešení přešel, protože ti nešlo přidávat do toho vektoru, tak to bylo tím, že jsi přidával do parametru, který je kopií původní hodnoty. Stačí ho deklarovat jako referenci a změna se promítne i ven, stejně jako to máš teď u 'pocet'.

void pridatRostlinu(Zahon& zahon)

Celkově je lepší všechny složitější struktury předávat jako referenci, aby se zbytečně nevytvářela kopie. A pokud ve funkci ten parametr nechceš měnit, tak ještě můžeš přidat 'const'.

A pokud zůstaneš u toho řešení s polem, tak ještě nezapomeň to staré vždycky ještě smazat (delete[])

#26 gna
Nechám si to pole, přijde mi to lepší .. na delete jsem samozřejmě zase zapomněl :) Děkuji

Netušíte někdo proč se mi data do souboru zapíší, ale neuloží ? po znovu spuštění programu se soubor vymaže, stejně tak pokud chci ukládat více než jednou (to už se ani nezapíší)

metoda ve třídě:

  void ulozDoSouboru(ofstream& soubor)
    {
        soubor << druh;
        soubor << barvaKvetu;
        soubor << vyska;
    }

implementace

ofstream f("cesta k souboru");

if (f.is_open())
                    {
                        for (int i = 0; i < pocet; i++)
                        {
                            rostliny[i].ulozDoSouboru(f);
                        }
                        cout << "Informace o rostlinach na zahone byly uspesne ulozeny. " << endl;
                        f.close();
                    }
                    else
                        cout << "Soubor se nepodarilo otevrit." << endl;
                }

#28 Yenkee
jak vis, ze se data do souboru zapisou, ale neulozi, kdyz se neulozi a nemuzes se tedy mrknout, ze v souboru opravdu jsou?

#29 KIIV

Pokud spustím program, přidám rostliny a uložím, tak jsou v souboru zapsány, pokud při stejném běhu programu přidám další rostlinu  a chci uložit podruhé, do souboru se už neuloží, jsou tam jen ty z prvního uložení (hlásí chybu, že se soubor nepodařilo otevřít). No a pokud program vypnu a znovu zapnu, tak se obsah souboru vymaže.

#30 Yenkee
To bude asi tím, že ten soubor na konci zavíráš. Do zavřeného souboru už nezapíšeš.

#31 Kit
pokud ho tedy pokazde neotevre

#30 Yenkee
bez kodu je to proste tezke odhadnout. Kazdopadne ofstream defaultne maze obsah, takze pokud to nemas spravne v seznamu, tak uz nic dalsiho nevykouzlis.

#31 Kit

Tu implementaci ukládání mám pod casem, čili pokud chce uživatel uložit, měl by se kód provést znovu celý ne ? tedy znovu otevřít soubor, zapsat a zavřít. Nebo to chápu špatně ?

#32 KIIV
Jak ten ofstream použít, aby soubor nepřemazával ?

#34 Yenkee
v tom myslim problem nebude, zameril bych se na to, co mas v seznamu. Umis to uz vypsat na obrazovku? Vypisuje to co ma ci ne?

#35 KIIV

Ano, mám dva druhy výpisů, kde každý používám na něco jiného (oba fungují, jak mají)

  void vypis()
    {
        cout << "ROSTLINA: " << druh << " "
             << ", BARVA KVETU: " << barvaKvetu << " "
             << ", VYSKA: " << vyska << "" << endl;
    }

void RostlinaInfo()
    {
        cout << "----------------------" << endl;
        cout << "Informace o rostline: " << endl;
        cout << "----------------------" << endl;
        cout << "Druh: " << druh << endl;
        cout << "Barva Kvetu: " << barvaKvetu << endl;
        cout << "Vyska: " << vyska << " cm" << endl;
        cout << "Cena: " << cenaDruhu() + cenaBarvyKvetu() + cenaVysky() << " Kc" << endl;
    }

#36 Yenkee
To je jedna rostlina, zkus je vypsat všechny. Problém bude v tom seznamu (nebo počtu), jinak to vypadá dobře.

#37 gna

všechny vypisuji opět pod casem

   case 3:
                    system("cls");
                    if (pocet < 1)
                    {
                        cout << "Zahon je prazdny ! Nejprve je treba nejakou rostinu pridat."
                             << endl;
                    }
                    for (int i = 0; i < pocet; i++)
                        rostliny[i].RostlinaInfo();

                    break;