Vektoren vergleich in C
-
'c' und 'd' werden eingesetzt, wenn amountv1<amountv2 bzw. amountv1>amountv2 ergibt
-
Tergo schrieb:
Ich dachte eher, dass Hauptaugenmerk liegt auf der linken Seite.
Weil da auch der Compiler-Output rauskommt, wa? Oh, nee, warte, das war der Input ...
Tergo schrieb:
Die einzige Warnung, die dann kam, drehte sich um das result==c und result==d, welches ich nun behoben habe. Die Berechnung wird jedoch weiterhin nicht richtig ausgeführt. Gcc Godbolt spuckt keine weiteren Warnungen aus.
Joa, das heißt halt, dass dein Programm syntaktisch korrekt ist, aber semantisch halt nicht. Sprich, du hast noch Fehler drinne, die dafür sorgen, dass es nicht so läuft, wie beabsichtigt.
Der Compiler kann dich vor einigen Fehlern warnen, aber bei weitem nicht vor allen.
Tergo schrieb:
'c' und 'd' werden eingesetzt, wenn amountv1<amountv2 bzw. amountv1>amountv2 ergibt
Die Frage war "Wann werden diese gesetzt?", nicht "eingesetzt".
Du verwendest diese Variablen, ohne ihnen einen Wert zugewiesen zu haben. Globale und statische Variablen werden immer mit 0 initialisiert, d.h. dass du in jedem Fall immer und immer 0 zurückgibst incompVecLen
.Ich würde deinem Dozenten mal auf den Tisch kotzen, wenn ich du wäre, offensichtlich weißt du nicht, was du tust.
-
Entschuldigung, hatte mich verschrieben, ich meinte die rechte Seite..
Kann ich den Fehler beheben indem ich die if Statements der Funktion wie folgt ändere?:
if(amountv1<amountv2) { return result=c; } if(amountv1>amountv2) { return result=d; } else { return 0; }
-
Tergo schrieb:
Kann ich den Fehler beheben indem ich die if Statements der Funktion wie folgt ändere?
Du liest meinen Text nicht durch. Du weißt
c
undd
einfach nie zu. Und deinif-else
-Konstrukt ist immer noch kaputt, das sollteif(x) {bla1} else if(y) {bla2} else {bla3}
sein.
-
Der Code ist jetzt fertig und deutlich entschlackt. Das Programm gibt richtige Ergebnisse raus, habe ich noch irgendwas vergessen? Vielen lieben Dank für die Hilfe bereits im Voraus!
#include <stdio.h> #include <math.h> int compVecLen(int v1[], int v2[]); int result; int main() { int V1[3]; int V2[3]; printf("Enter the values of Vector 1"); scanf("%d",&V1[0]); scanf("%d",&V1[1]); scanf("%d",&V1[2]); printf("Enter the values of Vector 2"); scanf("%d",&V2[0]); scanf("%d",&V2[1]); scanf("%d",&V2[2]); result=compVecLen(V1,V2); if(result == 0) { printf("|V1| = |V2|"); } else if(result == 1) { printf("|V1| > |V2|"); } else { printf("|V1| < |V2|"); } return 0; } int compVecLen(int v1[], int v2[]) { int amountv1=sqrt(v1[0]*v1[0] + v1[1]*v1[1] + v1[2]*v1[2]); int amountv2=sqrt(v2[0]*v2[0] + v2[1]*v2[1] + v2[2]*v2[2]); if(amountv1<amountv2) { return -1; } if(amountv1>amountv2) { return 1; } if(amountv1==amountv2) { return 0; } }
-
int main()
macht man so in C++. In C sollte esint main(void)
sein. Jede Funktion, die keinen Parameter übernimmt, solltevoid
in der Parameterliste haben.result
ist immer noch global, sollte lokal inmain
definiert werden.
- fehlende Einrückung.compVecLen
geht kürzer:
int compVecLen(int v1[], int v2[]) { int amountv1 = sqrt(v1[0]*v1[0] + v1[1]*v1[1] + v1[2]*v1[2]); int amountv2 = sqrt(v2[0]*v2[0] + v2[1]*v2[1] + v2[2]*v2[2]); if(amountv1 < amountv2) return -1; if(amountv1 > amountv2) return 1; return 0; }
sqrt
gibt einendouble
zurück, keinenint
. Das könnte dazu führen, dass Vektoren, die nicht gleichwertig sind, dennoch gleichwertig angesehen werden.
-
Habe int main() gelassen, weil wir es während der Vorlesung und im Praktikum immer so stehen haben lassen, zumindest bisher. Die Einrückungen kommen dann auf das finale Papier!
#include <stdio.h> #include <math.h> int compVecLen(int v1[], int v2[]); int main() { int result; int V1[3]; int V2[3]; printf("Enter the values of Vector 1"); scanf("%d",&V1[0]); scanf("%d",&V1[1]); scanf("%d",&V1[2]); printf("Enter the values of Vector 2"); scanf("%d",&V2[0]); scanf("%d",&V2[1]); scanf("%d",&V2[2]); result=compVecLen(V1,V2); if(result == 0) { printf("|V1| = |V2|"); } else if(result == 1) { printf("|V1| > |V2|"); } else { printf("|V1| < |V2|"); } return 0; } int compVecLen(int v1[], int v2[]) { double amountv1=sqrt(v1[0]*v1[0] + v1[1]*v1[1] + v1[2]*v1[2]); double amountv2=sqrt(v2[0]*v2[0] + v2[1]*v2[1] + v2[2]*v2[2]); if(amountv1<amountv2) { return -1; } if(amountv1>amountv2) { return 1; } return 0; }
-
Noch ein Hinweis, warum dein 2. korrigierter Code mit der Zuweisung nicht so funktioniert hat -> du mußt die Zuweisung umdrehen (denn du möchtest ja das Array mit den Werten X1, X2 etc. initialisieren):
V1[0] = X1; V1[1] = X2; // ...
Abschließend noch die Variante mit der direkten Initialisierung nach der Eingabe:
printf("Enter the values of Vector 1"); scanf("%d",&X1); scanf("%d",&X2); scanf("%d",&X3); printf("Enter the values of Vector 2"); scanf("%d",&Y1); scanf("%d",&Y2); scanf("%d",&Y3); /* Blockklammern für C89/C90 - nicht nötig bei C99 */ { int V1[3]={X1,X2,X3}; int V2[3]={Y1,Y2,Y3}; result = compVecLen(V1,V2); // ... }
-
Vielen lieben Dank
Wenn es dann da so steht, scheint alles so verdammt logisch.. Bei der nächsten Aufgabe werde ich auf solche Fehler besonders Acht geben!
-
Tergo schrieb:
Die Einrückungen kommen dann auf das finale Papier!
Darf man fragen warum? Einrückungen sollen dem Programmierer das Lesen des Codes leichter machen und nicht nur weil es andere Leute so wollen.
-
dachschaden schrieb:
compVecLen
geht kürzer:
int compVecLen(int v1[], int v2[]) { int amountv1 = sqrt(v1[0]*v1[0] + v1[1]*v1[1] + v1[2]*v1[2]); int amountv2 = sqrt(v2[0]*v2[0] + v2[1]*v2[1] + v2[2]*v2[2]); if(amountv1 < amountv2) return -1; if(amountv1 > amountv2) return 1; return 0; }
Geht sogar noch kürzer mit dem Ternary OP, oder sogar Branch free.
-
Andromeda schrieb:
Geht sogar noch kürzer mit dem Ternary OP, oder sogar Branch free.
Sobald du auch nur einmal den Optimizer drüberlaufen lässt, produziert der Compiler exakt den gleichen Maschinencode (wir reden nicht mal von hohen Stufen; -O1 genügt). Kürzer, vielleicht. Lesbarer?
#if 0 if(amountv1 < amountv2) return -1; if(amountv1 > amountv2) return 1; return 0; #else return (amountv1 < amountv2 ? -1 : (amountv1 > amountv2 ? 1 : 0)); #endif
Finde die erste Variante deutlich lesbarer. Also bleib bei der ersten Variante, es sei denn, der Optimizer fällt einem wirklich auf den Fuss.
-
dachschaden schrieb:
Andromeda schrieb:
Geht sogar noch kürzer mit dem Ternary OP, oder sogar Branch free.
Sobald du auch nur einmal den Optimizer drüberlaufen lässt, produziert der Compiler exakt den gleichen Maschinencode (wir reden nicht mal von hohen Stufen; -O1 genügt). Kürzer, vielleicht. Lesbarer?
#if 0 if(amountv1 < amountv2) return -1; if(amountv1 > amountv2) return 1; return 0; #else return (amountv1 < amountv2 ? -1 : (amountv1 > amountv2 ? 1 : 0)); #endif
Finde die erste Variante deutlich lesbarer. Also bleib bei der ersten Variante, es sei denn, der Optimizer fällt einem wirklich auf den Fuss.
Probier mal das:
return (((amountv2-amountv1)<1)-1)|(amountv1!=amountv2);
-
Andromeda schrieb:
Probier mal das:
Schlechter lesbar, produziert noch mehr Maschinencode, und die Jumps sind immer noch nicht weg:
movapd xmm0,xmm1 mov QWORD PTR [rsp+0x8],rsi call 400650 <sqrt@plt> mov rsi,QWORD PTR [rsp+0x8] jmp 40088c <compVecLen+0x2c> movapd xmm0,xmm1 call 400650 <sqrt@plt> jmp 4008b7 <compVecLen+0x57>
Diesen Block hast du in beiden Varianten, nur dein Code macht noch mehr Zeug, was den Code nur aufbläht.
-
dachschaden schrieb:
int main()
macht man so in C++. In C sollte esint main(void)
sein. Jede Funktion, die keinen Parameter übernimmt, solltevoid
in der Parameterliste haben.
Quatsch.
-
dachschaden schrieb:
Andromeda schrieb:
Probier mal das:
Schlechter lesbar, produziert noch mehr Maschinencode, und die Jumps sind immer noch nicht weg:
movapd xmm0,xmm1 mov QWORD PTR [rsp+0x8],rsi call 400650 <sqrt@plt> mov rsi,QWORD PTR [rsp+0x8] jmp 40088c <compVecLen+0x2c> movapd xmm0,xmm1 call 400650 <sqrt@plt> jmp 4008b7 <compVecLen+0x57>
Bei mir sind keine calls und jmps dabei:
10: return (((amountv2-amountv1)<1)-1)|(amountv1!=amountv2); 004010E2 mov ecx,dword ptr [ebp-8] 004010E5 sub ecx,dword ptr [ebp-4] 004010E8 xor eax,eax 004010EA cmp ecx,1 004010ED setl al 004010F0 sub eax,1 004010F3 mov edx,dword ptr [ebp-4] 004010F6 xor ecx,ecx 004010F8 cmp edx,dword ptr [ebp-8] 004010FB setne cl 004010FE or eax,ecx
-
Andromeda schrieb:
return (((amountv2-amountv1)<1)-1)|(amountv1!=amountv2);
Nonsens.
Bitoperatoren auf negative int loszulassen ist einfach nur dumm und ahnlungslos.
-
Wutz schrieb:
Andromeda schrieb:
return (((amountv2-amountv1)<1)-1)|(amountv1!=amountv2);
Nonsens.
Bitoperatoren auf negative int loszulassen ist einfach nur dumm und ahnlungslos.Dann check mal diese hübsche Variante:
return ((amountv2-amountv1)<0) - ((amountv2-amountv1)>0);
-
Wutz schrieb:
dachschaden schrieb:
int main()
macht man so in C++. In C sollte esint main(void)
sein. Jede Funktion, die keinen Parameter übernimmt, solltevoid
in der Parameterliste haben.
Quatsch.
Daniel Earwicker schrieb:
Means different things in C and C++! In C it means "could take any number of parameters of unknown types", and in C++ it means the same as foo(void).
Variable argument list functions are inherently un-typesafe and should be avoided where possible.
Und wenn man sich nicht daran gewöhnen will, dann gibt man überall konsequent
(void)
an. Inwiefern das Quatsch ist, musst du mal erklären.Andromeda schrieb:
Bei mir sind keine calls und jmps dabei
Schau dir den kompletten Maschinencode der Funktion an. In dem Teil, in dem ich die alte Variante drin hab, sind am Ende auch keine Jumps mehr, aber in der Funktion generell bleiben sie noch.
Und das einzige, was dein Code macht, ist den Endcode weiter aufzublähen.
Andromeda schrieb:
Dann check mal diese hübsche Variante
Das einzige, was man dieser Variante zugutehalten kann, ist dass der Compiler hier nicht rbx zwischenspeichert -
obwohl er theoretisch auch das sub rsp,0x18 entfernen könnte. Red Zone und so (auf Linux, mit GCC kompiliert).* Ansonsten hat der gesamte Code noch mehr Calls und Jumps drin als zuvor:sub rsp,0x18 mov ecx,DWORD PTR [rdi] mov edx,DWORD PTR [rdi+0x4] mov eax,DWORD PTR [rdi+0x8] pxor xmm0,xmm0 imul ecx,ecx imul edx,edx imul eax,eax add edx,ecx add eax,edx cvtsi2sd xmm0,eax sqrtsd xmm2,xmm0 ucomisd xmm2,xmm2 jp 4008d0 <compVecLen+0x70> mov ecx,DWORD PTR [rsi] mov edx,DWORD PTR [rsi+0x4] mov eax,DWORD PTR [rsi+0x8] pxor xmm0,xmm0 imul ecx,ecx imul edx,edx imul eax,eax add edx,ecx add eax,edx cvtsi2sd xmm0,eax sqrtsd xmm1,xmm0 ucomisd xmm1,xmm1 jp 4008e5 <compVecLen+0x85> cvttsd2si eax,xmm2 cvttsd2si edx,xmm1 sub edx,eax mov eax,edx shr eax,0x1f test edx,edx setg dl add rsp,0x18 movzx edx,dl sub eax,edx ret mov QWORD PTR [rsp+0x8],rsi call 400650 <sqrt@plt> mov rsi,QWORD PTR [rsp+0x8] movapd xmm2,xmm0 jmp 40088b <compVecLen+0x2b> movsd QWORD PTR [rsp+0x8],xmm2 call 400650 <sqrt@plt> movsd xmm2,QWORD PTR [rsp+0x8] movapd xmm1,xmm0 jmp 4008b2 <compVecLen+0x52> nop DWORD PTR [rax+0x0]
EDIT:
sub
eingefügt.EDIT:
*Ach, nee, ich bin blöd. In der Funktion wird ein Call zusqrt
gemacht, welches sich an das ABI halten muss, und die resetten den Stack natürlich. Red Zone geht nur in Leaf-Funktionen.
-
dachschaden schrieb:
Schau dir den kompletten Maschinencode der Funktion an. In dem Teil, in dem ich die alte Variante drin hab, sind am Ende auch keine Jumps mehr, aber in der Funktion generell bleiben sie noch.
Der Aufruf der sqrt-Funktion braucht sicherlich Calls, aber Jumps finde ich schon ziemlich schräg, wenn einfach nur straight durchgerechnet wird.
Aber gut, Compilerentwickler wissen was sie tun. Sollte man jedenfalls meinen.