[C++] getallen op grootte sorteren werkt niet?

[Akropolis]

Ik moet voor school een progje monteren die vraagt om drie getallen en deze vervolgens van groot naar klein op het scherm zet.
Nu ben ik hier al een tijd mee bezig, maar ik krijg het niet goed werkend.

Dit heb ik gemaakt:

C:

#include <iostream.h>

void main()
{
 int getal1, getal2, getal3, uitkomst1, uitkomst2, uitkomst3, qwe1; //declareer getal1 t/m 3 en uitkomst1 t/m 3 en qwe1


 cout << "Geef getal 1: "; //schrijf "Geef getal 1: "
 cin >> getal1; //zet getal in getal1

 cout << "Geef getal 2: "; //schrijf "Geef getal 1: "
 cin >> getal2; //zet getal in getal2

 cout << "Geef getal 3: "; //schrijf "Geef getal 1: "
 cin >> getal3; //zet getal in getal3

 
 if (getal1 > getal2) uitkomst1 = getal1, qwe1 = 0; else uitkomst1 = getal2, qwe1 = 1; //kijk of getal 1 groter is dan getal2 en zet de grootste in uitkomst1 en geef qwe1 de juiste waarde
 if (uitkomst1 < getal3) uitkomst1 = getal3, qwe1 = 2; //kijk of uitkomst1 groter is dan getal3 en geef e.v.t qwe1 een andere waarde
 
 //uitkomst1 is altijd de grootste

 if (qwe1 = 0) goto A; //als qwe1 0 is ga naar A
 if (qwe1 = 1) goto B; //als qwe1 1 is ga naar B
 if (qwe1 = 2) goto C; //als qwe1 2 is ga naar C
 
 A:
 if (getal2 > getal3) uitkomst2 = getal2, uitkomst3 = getal3; else uitkomst2 = getal3, uitkomst3 = getal2; //kijk of getal2 groter is dan getal3 en ze de grootste in uikomst2 en de kleinste in uikomst3
 goto Z; //ga naar Z
 
 B:
 if (getal1 > getal3) uitkomst2 = getal1, uitkomst3 = getal3; else uitkomst2 = getal3, uitkomst3 = getal1; //kijk of getal1 groter is dan getal3 en ze de grootste in uikomst2 en de kleinste in uikomst3
 goto Z; //ga naar Z

 C:
 if (getal1 > getal2) uitkomst2 = getal1, uitkomst3 = getal2; else uitkomst2 = getal2, uitkomst3 = getal1; //kijk of getal1 groter is dan getal2 en ze de grootste in uikomst2 en de kleinste in uikomst3

 Z:
 cout << "Dit zijn de getallen van groot naar klein: " << uitkomst1 << " - " << uitkomst2 << " - " << uitkomst3; //geef ge getallen weer van groot naar klein

 cin.get(); 
 cin.get(); 
}

Het probleem is dat getal2 soms door getal1 of getal3 wordt vervangen?
Hier een paar combinaties:

Getal1: 1
Getal2: 2
Getal3: 3
Uitkomst: 3 - 3 -1

Getal1: 3
Getal2: 2
Getal3: 1
Uitkomst: 3 - 3 -1

Getal1: 2
Getal2: 3
Getal3: 1
Uitkomst: 3 - 2 -1 (dit is dus wel goed)

Getal1: 1
Getal2: 3
Getal3: 2
Uitkomst: 3 - 2 -1 (dit is dus wel goed)

Dus als getal2 de grootste is werkt het, anders niet.
Ik kan de fout nergens vinden, en ik kan het ook niet opzoeken want ik weet ook niet wat de fout is.
Iemand een idee waar het aan kan liggen of hoe dit komt?

(ik ben een C++ n00b dus vandaar de voor jullie waarschijnlijk omslachtige lap tekst )

[.oisyn]

begin eerst eens met het goed indenteren van je if-blocks, want dit noem ik nou niet bepaald leesbaar.
Zo maak je een if

C++:

if (conditie)
    regel_code;
else
    regel_code;

// of natuurlijk met compound statements:
if (conditie)
{
    regel_code;
    regel_code;
    regel_code;
}
else
{
    ...
}

en goto hoor je al helemaal niet te gebruiken


Verder is het misschien handiger om de uitkomsten in een array te gooien en die te sorteren met std::sort

[xos]

Ben ook een C++ nOOb, java ligt meer in mijn straatje. Maar ik zou het in een array(list) gooien en het zaakje sorteren. Je huidige methode is hmm, behoorlijk omslachtig Het gaat nu om 3 getallen maar mocht je later hetzelfde doen met 1000 nummers wordt het nl al een ander verhaal.

[Verwijderd]

C++:

//initialisatie
a=0;
b=0;
c=0;
--inlezen gettallen--
if (a>b){
  if(b>c){
     print a;
     print b;
     print c;
  }else{
     print a;
     print c;
     print b;
  }
}else....

ga daar es mee verder, het kan nog veel vetter maar dat is een beetje over the top

[.oisyn]

3 getallen sorteren kun je overigens met 3 if statements en swaps

C++:

if (g1 > g2)
    swap (g1, g2);
if (g2 > g3)
    swap (g2, g3);
if (g1 > g2)
    swap (g1, g2);

[Verwijderd]

en een 1337 swap functie (c) mijn leraar programmeerconcepten:

C:

void intSwap(int *a, int *b){
        *a-=(*a+=*b,*b=*a-*b);
    }

[Verwijderd]

Je probleem zit hem in

code:

if (qwel = 0) ...

etc. Waarschijnlijk bedoel je

code:

if (qwel == 0)

[SuperRembo]

basterd schreef op 28 september 2002 @ 17:17:
Ben ook een C++ nOOb, java ligt meer in mijn straatje. Maar ik zou het in een array(list) gooien en het zaakje sorteren. Je huidige methode is hmm, behoorlijk omslachtig Het gaat nu om 3 getallen maar mocht je later hetzelfde doen met 1000 nummers wordt het nl al een ander verhaal.

Gezien de opdracht lijkt me dat arrays nog niet behandeld zijn.

[Feyd-Rautha]

Om 3 getallen te sorteren kun je natuurlijk met een IF-structuur gemakkelijk het probleem oplossen.

Het is misschien wel beter om een Sorteer-algoritme te gebruiken. Indien je opeens een array met 1242 getallen hebt, heb je maar uw algoritme te gebruiken en klaar is kees...

[Akropolis]

w031, bedankt John_Smith, dat was idd het probleem . Het werkt nu dus.
En ik heb .iosyn zijn tips ook maar een beetje opgevolgt .
Alleen met arrays enz (zover ben ik nog niet want ik heb nog niet eens een boek waar ik normaal mee kan werken)
Maar nog 1 vraag, waarom geen goto gebruiken .iosyn?

[MSalters]

Akropolis schreef op 28 september 2002 @ 17:29:
w031, bedankt John_Smith, dat was idd het probleem . Het werkt nu dus.
En ik heb .iosyn zijn tips ook maar een beetje opgevolgt .
Alleen met arrays enz (zover ben ik nog niet want ik heb nog niet eens een boek waar ik normaal mee kan werken)
Maar nog 1 vraag, waarom geen goto gebruiken .iosyn?

"Go to considered harmful", Prof E. Dijkstra, 1968.

[Ericston]

Verwijderd schreef op 28 september 2002 @ 17:22:
en een 1337 swap functie (c) mijn leraar programmeerconcepten:

C:

void intSwap(int *a, int *b){
        *a-=(*a+=*b,*b=*a-*b);
    }

Als we het daarover gaan hebben...

C++:

void anotherIntSwap(int &a, int &b) {
    a ^= b;
    b ^= a;
    a ^= b;
}

[.oisyn]

hmm

C++:

template<typename T> inline void swap (T & t1, T & t2)
{
    T temp (t1);
    t1 = t2;
    t2 = temp;
}

[Ericston]

.oisyn schreef op 28 september 2002 @ 19:14:
hmm

C++:

template<typename T> inline void swap (T & t1, T & t2)
{
    T temp (t1);
    t1 = t2;
    t2 = temp;
}

Hoogst inefficient.

offtopic:
Irri he, die htmlrechten? Wrom wordt & 2x vervangen? Nu staat er &.

[Tim]

Waarom zoveel comments? het is een vrij simpel stukje code, en die comments voegen eigenlijk niks toe

[Ericston]

Ze voegen niet zoveel meer toe aan het originele topic, maar ze hadden het potentieel om op een leuke discussie over code efficientie uittelopen. Het is toch niet zo dat je niet meer mag reageren als het probleem is opgelost? Dit is nog altijd een discussieforum en geen helpdesk.

Juist jouw reply voegt wat dat betreft niks toe.

[Tim]

Ericston schreef op 28 september 2002 @ 20:03:
Ze voegen niet zoveel meer toe aan het originele topic, maar ze hadden het potentieel om op een leuke discussie over code efficientie uittelopen. Het is toch niet zo dat je niet meer mag reageren als het probleem is opgelost? Dit is nog altijd een discussieforum en geen helpdesk.

Juist jouw reply voegt wat dat betreft niks toe.

Ik had het over de comments in de code:

cout << "Geef getal 1: "; //schrijf "Geef getal 1: "
cin >> getal1; //zet getal in getal1

[Verwijderd]

hmmm.... interessant.

GAS:

push AX
mov AX,BX
pop BX

ik weet niet wat sneller is, dit of Ericstons variant

GAS:

xor AX,BX
xor BX,AX
xor AX,BX

Dat zal dus van de hardware afhangen

edit:
net ff gestest in VC++6, Ericstons variant is idd sneller

[Verwijderd]

Laten we het er maar op houden dat de efficientie van je swap-functie er IRL niet veel toe doet. Hier swapped ie 1 miljard getallen in 3.5 seconden (5 seconden met mijn variant). Da's een aardig verschil, maar hoevaak moet je nou 1 miljard getallen swappen???

[Ericston]

Timpie schreef op 28 september 2002 @ 20:37:
[...]
Ik had het over de comments in de code:
[...]

ROFL

Sorry.

Verwijderd schreef op 28 september 2002 @ 21:05:
Laten we het er maar op houden dat de efficientie van je swap-functie er IRL niet veel toe doet. Hier swapped ie 1 miljard getallen in 3.5 seconden (5 seconden met mijn variant). Da's een aardig verschil, maar hoevaak moet je nou 1 miljard getallen swappen???

Ah, een benchmark. Nice.
Ben het met je eens dat dit meer meer onder programmeurs "bijgeloof" valt dan dat je je er serieus druk om zou moeten maken.

[Verwijderd]

Verwijderd schreef op 28 september 2002 @ 21:05:
Laten we het er maar op houden dat de efficientie van je swap-functie er IRL niet veel toe doet. Hier swapped ie 1 miljard getallen in 3.5 seconden (5 seconden met mijn variant). Da's een aardig verschil, maar hoevaak moet je nou 1 miljard getallen swappen???

Plaagstootje: Heb je wel eens grote arrays gesorteerd?

[Akropolis]

Timpie schreef op 28 september 2002 @ 19:58:
Waarom zoveel comments? het is een vrij simpel stukje code, en die comments voegen eigenlijk niks toe

Die uitleg staat er bij omdat dat moet van mijn docent programmeren

[.oisyn]

Verwijderd schreef op 28 september 2002 @ 20:59:
hmmm.... interessant.

GAS:

push AX
mov AX,BX
pop BX

ik weet niet wat sneller is, dit of Ericstons variant

GAS:

xor AX,BX
xor BX,AX
xor AX,BX

Dat zal dus van de hardware afhangen

edit:
net ff gestest in VC++6, Ericstons variant is idd sneller

en daarvoor hebben ze de xchg instructie uitgevonden
sowieso is gebruik maken van de stack niet echt een slimme optie, daar kun je ook wel een extra register voor gebruiken (vooropgesteld dat je er 1 vrij hebt op dat moment natuurlijk)

[curry684]

Akropolis schreef op 28 september 2002 @ 17:29:
Maar nog 1 vraag, waarom geen goto gebruiken .iosyn?

Omdat het in tegenstelling tot reguliere methodes om hetzelfde effect te gebruiken de compiler schier onmogelijk maakt te optimaliseren, en omdat het tegen ieder standaard programmeerconcept in gaat, en omdat de leesbaarheid en onderhoudbaarheid van je code erdoor achteruit denderen, en omdat je ontslagen wordt bij het eerste bedrijf waar je het toepast, en puur om het principe dat het van lelijke programmeerstijl getuigt.

Er is geen enkele toepassing van goto die je niet netter kunt bouwen, en daar houd je wel snellere en beter onderhoudbare code aan over. Not done dus, goto is gewoon een fout van de taal die er voor C-backwards-compatibility inzit en daarin ook al fout was.

Had ik al 'not done' gezegd?

[MSalters]

curry684 schreef op 30 september 2002 @ 08:46:
Not done dus, goto is gewoon een fout van de taal die er voor C-backwards-compatibility inzit en daarin ook al fout was.

Neuh, goto zit er in voor lex/yacc en dergelijke tools. Dat wil zeggen, je mag het gebruiken in C code waar mensen niet naar kijken. C is af en toe veredelde assembly, en goto is daarin de portable manier om een JMP te coden.

[Verwijderd]

Plaagstootje: Heb je wel eens grote arrays gesorteerd?

Moet je geen bubblesort gebruiken
met quicksort heb je niet veel swaps (relatief dan). En over wat voor array zou je het hier willen hebben dan? >100M getallen sorteren, ja dan misschien, en dan duur het dus in DEBUG MODE (release mode executable is nog ong. 2x zo snel) 3.5 seconden...

Ook iets om over na te denken : enig idee hoeveel al dat geheugen kost om die array in te stoppen? zeg 1G getallen x 32bits = 4GB mem, alleen voor je array
Als je zoveel geld hebt dat je dat kan gebruiken kan je ook wel een degelijke power4 voor dit soort werk nemen ipv mijn "armzalige" Athlon XP 1600+

[MisterData]

Nah de vraag van de topicstart is iig opgelost

[Verwijderd]

Offtopic:
Hehe, ik heb ooit in de duistere middeleeuwen nog leren programmerin in basic (ja ik weet 't, ik mag niet vloeken ) en daar deed je zowat niks anders als goto's. Een normale functie aanroepen zat er nog niet eens in (alleen simpele inline functies). Werk nu in C++, maar heb aan dat basic wel een hartgrondige hekel aan goto's overgehouden. In die zin was het in elk geval nuttig...

[Verwijderd]

Verwijderd schreef op 30 september 2002 @ 16:54:
Moet je geen bubblesort gebruiken
met quicksort heb je niet veel swaps (relatief dan).

Valt wel mee hoor, gemiddeld n * ln(n); in het slechtst mogelijke geval n², maar daar gaat het niet om. Bij alle in-place sorteeralgoritmes is het swappen van elementen de duurste operatie, vandaar ook dat men het aantal swaps probeert te reduceren.

[MSalters]

Verwijderd schreef op 30 september 2002 @ 17:25:
[...]

Valt wel mee hoor, gemiddeld n * ln(n); in het slechtst mogelijke geval n², maar daar gaat het niet om. Bij alle in-place sorteeralgoritmes is het swappen van elementen de duurste operatie, vandaar ook dat men het aantal swaps probeert te reduceren.

Da's weer een geval van overgeneralisatie. Ik heb hier toch een probleem gehad warbij de sorteerfunctie beperkt werd doordat de compare functie een dure transformatie moest aanroepen (O(NlogN) in element data); de uiteindelijke swap was O(N).
Evengoed was een bubblesort daar pijnlijk geweest. Ik denk dat elk redelijk sorteeralgoritme op random data een 50% kans heeft om elementen te swappen, dwz de compare zal net zo vaak true als false retourneren. Dus bubblesort roept ook veel te vaakde compare aan.

( De uiteindelijke oplossing was om een partiele sorteervolgorde te cachen; alleen in de 1% gevallen waarin die hetzelfde was moet je dan een compare doen, de data overhead per element is O(1). )