[C++] betere methode?

[Verwijderd]

hallo

Ik ben momenteel bezig met c++ te leren (tutorials, boeken) en ik besloot een simpel rekenmachientje te maken

hierbij de code

C++:

 #include <iostream>
using namespace std;

int add(int x, int y) {   // function declarations
    return (x + y);
}

int min(int x, int y) {
    return (x - y);
}

int mult(int x, int y) {     
    return (x * y);
}

float division(int x, int y) {
    return ((float)x / (float)y);
}

int mod(int x, int y) {
    return (x % y);
}


int main() {   // main
    int n1,n2;
    int keuze;
    char opnieuw;
    
    do {
                             
    cout<<"Voer het eerste getal in: ";
    cin>>n1;
    cin.ignore();
    cout<<"voer het tweede getal in: ";
    cin>>n2;
    cin.ignore();
    
    cout<<"Voer je keuze in: ";
    cout<<"1 = vermeningvuldiging\n";
    cout<<"2 = delen\n";
    cout<<"3 = rest berekenen\n";
    cout<<"4 = optellen\n";
    cout<<"5 = aftrekken\n";
    
    cin>>keuze;  //invoer van keuze
    cin.ignore();  //verwijderen van enter
    switch ( keuze ) {
           case 1:
                cout<<mult(n1,n2)<< endl;
                break;
           case 2:
                cout<<division(n1,n2)<< endl;
                break;
           case 3:
                cout<<mod(n1,n2)<< endl;
                break;
           case 4:
                cout<<add(n1,n2)<< endl;
                break;
           case 5:
                cout<<min(n1,n2)<< endl;
                break;
           default:
                cout<<"Ongeldige selectie!\n";
                cout<<"Voer je keuze in: ";
                continue;
                }
    
              
      do {
        cout<<"Wil je nog een keer (y/n)?\n";
        cin>>opnieuw;
        cin.ignore();
    
        if ((opnieuw == 'y') || (opnieuw == 'Y')) {
                 cout<<"ok\n";
                 break;
                 }
        else if ((opnieuw == 'n') || (opnieuw == 'N')) {
                 cout<<"bye bye\n";
                 break;
                 }
        else {
                 cout<<"ongeldige keuze\n";
        }
    
     } while (((opnieuw != 'y') || (opnieuw != 'Y')) || ((opnieuw != 'n') || (opnieuw != 'N')));
                           
    }  while (opnieuw == 'y' || opnieuw == 'Y');

getchar();  // pause
return 0;
    
}  // end main               

wat mij vooral stoort is het gebruik van die char opnieuw en die if's daarbij. Hoe kan ik dit het beste optimaliseren.

Alvast bedankt

[ Voor 37% gewijzigd door Verwijderd op 23-03-2005 19:19 ]

[MSalters]

Ten eerste: gebruik functies vor de niet-triviale dingen. Nu gebruik je alleen functies voor de triviale. Dat is dus precies verkeerd om. Iedereen weet wat x*y doet, als je daar mult(x,y) van maakt ben je nodeloos ingewikkeld aan het doen.
Schrijf je daarentegen

C++:

#include <iostream> 
void get_input_and_calc_result( std::ostream& );
bool ok_to_continue( std::ostream& );

int main( )
{
  do {
    get_input_and_calc_result( std::cout );
  } while (ok_to_continue( std::cout ) );
}
// ... en nu de _echte_ functies

dan is meteen duidelijk wat je programma doet. Hoe je dat dan doet staat opgesplitst over twee functies, die komen later wel.

[Robtimus]

Verwijderd schreef op woensdag 23 maart 2005 @ 18:30:

C++:

while (((opnieuw != 'y') || (opnieuw != 'Y')) || ((opnieuw != 'n') || (opnieuw != 'N')));

Dit levert dus altijd true op.

Even de overbodige haakjes weghalen:

C++:

while ((opnieuw != 'y') || (opnieuw != 'Y') || (opnieuw != 'n') || (opnieuw != 'N'));

Dus het is alleen false als (DeMorgen even toepassen):
(opnieuw == 'y') && (opnieuw == 'Y') && (opnieuw == 'n') && (opnieuw == 'N')

Dat gebeurt dus alleen maar als 'y' == 'n'

[ Voor 34% gewijzigd door Robtimus op 23-03-2005 23:10 ]

[Soultaker]

Ik doe dan meestal gewooon if(strchr("yYnN", opnieuw)) ...
Lijkt me een stuk leesbaarder.

[ Voor 13% gewijzigd door Soultaker op 23-03-2005 23:11 ]

[Voutloos]

Met de huidige constructie is regel 88 overbodig/lelijk (en inderdaad zelfs helemaal fout). Als je gewoon de do... while vervangt door een while(1), kan je na het printen van "ongeldige keuze\n" een continue doen. Scheelt 4 vergelijkingen en leest een stuk lekkerder.

edit:
Of Soultakers oplossing gebruiken, lees dan wel ff wat strchr precies doet zodat je er wat van leert, dat is sowieso een handige functie

Tevens een klein mierenneuk dingetje: min(int x, int y) geldt doorgaans als een functie welke het kleinste getal teruggeeft ipv een functie welke y van x aftrekt. Het is geen handig gekozen naam, substract() zou beter zijn. Nu is het nog een triviaal detail, maar als andere mensen je functies gaan gebruiken zorgt zoiets voor verwarring. Tevens zou je hier alsnog achterkomen als je ooit echt de daadwerlijke min() operatie aan je programma gaat toevoegen.

[ Voor 16% gewijzigd door Voutloos op 23-03-2005 23:20 ]

[Soultaker]

Hoe je het ook aanpakt, C++ standaard I/O is uiteindelijk gewoon niet geschikt (of bedoeld) voor interactieve invoer. Als je 'm toch gebruikt, heeft het sowieso de voorkeur om ofwel regels, ofwel 'eenheden' gescheiden door spaties te lezen; dus ints en strings effectief. Daarmee voorkom je dat je moeilijk moet doen om karakters te negeren (zoals nu met cin.ignore()).

[Verwijderd]

ok, bedankt voor de tips

[MSalters]

Voutloos schreef op woensdag 23 maart 2005 @ 23:16:
Tevens een klein mierenneuk dingetje: min(int x, int y) geldt doorgaans als een functie welke het kleinste getal teruggeeft ipv een functie welke y van x aftrekt. Het is geen handig gekozen naam, substract() zou beter zijn. Nu is het nog een triviaal detail, maar als andere mensen je functies gaan gebruiken zorgt zoiets voor verwarring. Tevens zou je hier alsnog achterkomen als je ooit echt de daadwerlijke min() operatie aan je programma gaat toevoegen.

Ehm, heeft hij dat niet gedaan ?
De echte functie is template<typename T> std::min(T,T) en die zit dus in een header, mogelijk zelfs in <iostream>. (NB C++ headers mogen elkaar includen). Vanwege de using namespace std; is
min(3L,2L) dus 2L (!)

[Soultaker]

Hmz, toch kan ik die code wel gewoon compileren, zelfs als ik expliciet <algorithm> include en dan selecteert 'ie (GCC) zonder warning of iets de zelfgedefinieerde min-functie. Ik krijg pas de STL-min wanneer ik expliciet min<int> schrijf.

Blijkbaar werkt dat automatisch invullen van template arguments (hoe heet dat?) pas als de functie als zodanig niet gevonden wordt, of is dit GCC-specifiek gedrag? (In dat geval had ik ten minste een warning verwacht.) Of misschien is het iets dat functies uit de default namespace voorang krijgen?

[ Voor 11% gewijzigd door Soultaker op 24-03-2005 19:54 ]

[Voutloos]

MSalters schreef op donderdag 24 maart 2005 @ 19:31:
[...]

Ehm, heeft hij dat niet gedaan ?

Laat ik het dan maar kort zeggen: Ik vind de naam min voor een functie ambigue.

[Verwijderd]

Zeker als de andere functienamen wel een Engelse naam hebben. Maar ik zou zeker veel meer functies gebruiken, vooral als je een beginnende programmeur bent.

[MSalters]

Voutloos schreef op donderdag 24 maart 2005 @ 20:02:
[...]
Laat ik het dan maar kort zeggen: Ik vind de naam min voor een functie ambigue.

Dat snap ik. Ik wilde er alleen op wijzen dat het meer dan een opinie of denkbeeldig risico is. Er is een andere functie met bijna dezelfde naam, en misschien is die toegevoegd. De ambiguiteit is subtiel, vanwege de using, vandaar dat ik daar expliciet op wees.

[MSalters]

Soultaker schreef op donderdag 24 maart 2005 @ 19:53:
Hmz, toch kan ik die code wel gewoon compileren, zelfs als ik expliciet <algorithm> include en dan selecteert 'ie (GCC) zonder warning of iets de zelfgedefinieerde min-functie. Ik krijg pas de STL-min wanneer ik expliciet min<int> schrijf.

Eh, niet in de buurt van compiler en ik wilde het niet al te complex maken. Met zo'n "shotgun" using namespace trek je alle namen van std:: in de namespace waar de using staat (in dit geval de globale, :: ) en daarna vind je die namen op de normale manier (3.4.1 unqualified name).

In dit geval vind je dus std::min en de TS z'n min. In mijn optiek zou overload resolution hier moeten kiezen tussen std::min<long>(long,long) en ::min(int,int). De eerste is beter; de conversion sequences zijn in dat geval twee keer de identity conversion long->long ipv long->int (integral conversion). Ik snap dus even niet waarom je niet de STL min<long> krijgt.

[ Voor 5% gewijzigd door MSalters op 24-03-2005 23:22 . Reden: [/quote] ]

[Soultaker]

Ow hmm, als de argumenten expliciet longs maakt, dan krijg je ook inderdaad de STL-variant. (Maar als 1 van de 2 long is, niet, terwijl je daar evenveel conversies zou verwachten.) Het is in ieder geval niet iets waar je over na wilt hoeven denken.

[MSalters]

Tja, als je std::min(1, 2L ) hebt, moet je dan std::min<int> of std::min<long> gebruiken? In beide gevallen is er een mismatch met een parameter, en dus worden geen van beide templates automatisch geinstantieerd. Dan blijft dus de ::min(int,int) over.

[Verwijderd]

Verwijderd schreef op donderdag 24 maart 2005 @ 22:52:
Zeker als de andere functienamen wel een Engelse naam hebben. Maar ik zou zeker veel meer functies gebruiken, vooral als je een beginnende programmeur bent.

wel ik dacht eigenlijk aan minus, maar kun je een voorbeeld geven van waar ik best functies zou gebruiken en niet bij die + - * /

[MSalters]

Verwijderd schreef op vrijdag 25 maart 2005 @ 11:20:
[...]
wel ik dacht eigenlijk aan minus, maar kun je een voorbeeld geven van waar ik best functies zou gebruiken en niet bij die + - * /

Lees mijn eerste rectie nog eens?