c++.de :: Code Optimierung/Verbesserung (Addition n-ziffern langer Zahlen)

rufrider Mitglied 19:36:52 09.06.2012 Titel:

Hey Leute,

wieder mal geht es um eine Testat Aufgabe. Jedoch habe Ich sieh diesmal
gelöst. Nun ist die Frage was kann ich am Code verbessern, was geht einfacher...

Aufgabe:

Code:
Erstellen Sie ein Programm, das zwei beliebig lange positive Zahlen einliest und die Summe dieser beiden Zahlen wieder ausgibt. Hinweise: „Beliebig lang“ geht nicht so einfach... Wählen Sie als Obergrenze für die Anzahl der Ziffern den Wert 100 Die Eingabe der Zahlen an der Konsole soll als Zeichenkette erfolgen!

Mein Code:

C++:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143	#include <stdio.h> #include <stdlib.h> #include <string.h> #define LIMIT 101 /* * / void benutzerEingabe(char z1, char z2); void addition(char z1, char z2, char erg); void ausgabe(char erg, int ergLaenge); int laengeBestimmen(char array, int arrayLaenge); //Falls eine zahl mehr Ziffern hat als die andere void arrayInhaltVerschieben(char array, int arrayLimit, int verschieben); int main(int argc, char* argv) { int zahlLaenge1, zahlLaenge2, ergLaenge; //Arrays Definieren char zahl1[LIMIT]; char zahl2[LIMIT]; char erg[LIMIT+1]; //Arrays Initialisieren memset(zahl1, 48, LIMIT); memset(zahl2, 48, LIMIT); memset(erg, 48, LIMIT+1); benutzerEingabe(zahl1, zahl2); zahlLaenge1 = laengeBestimmen(zahl1, LIMIT); zahlLaenge2 = laengeBestimmen(zahl2, LIMIT); //Ergebnislänge bestimmen und kleinere zahl der größeren Stellenweise anpassen if(zahlLaenge1<=zahlLaenge2) { if(zahlLaenge2-zahlLaenge1>0) arrayInhaltVerschieben(zahl1, LIMIT, (zahlLaenge2-zahlLaenge1)); ergLaenge = zahlLaenge2; } else { if(zahlLaenge1-zahlLaenge2>0) arrayInhaltVerschieben(zahl2, LIMIT, (zahlLaenge1-zahlLaenge2)); ergLaenge = zahlLaenge1; } //-NEWLINE- ans Arrayende setzen und vorige mit 48(ASCII) auffüllen int j = 0; for(j; j<LIMIT; j++) { if(zahl1[j] == 10) zahl1[j] = 48; if(zahl1[j] == 0) zahl1[j] = 48; if(zahl2[j] == 10) zahl2[j] = 48; if(zahl1[j] == 0) zahl1[j] = 48; } zahl1[LIMIT-1] = 10; zahl2[LIMIT-1] = 10; addition(zahl1, zahl2, erg); ausgabe(erg, ergLaenge); return (EXIT_SUCCESS); } void benutzerEingabe(char z1, char z2) { printf("Addition von zwei langen(100 ziffern) Zahlen."); printf("\n============================================="); printf("\nZahl eins eingeben: "); fgets(z1, LIMIT, stdin); while(getchar() != 10) getchar(); //Puffer leeren, wenn Zahl zu groß printf("Zahl zwei eingeben: "); fgets(z2, LIMIT, stdin); while(getchar() != 10) getchar(); //Puffer leeren, wenn Zahl zu groß } void addition(char z1, char z2, char erg) { int h1, h2, ueber, teilErg; ueber = 0; int i = LIMIT-1; for(i; i>=0; i--) { h1 = (int)z1[i]-48; h2 = (int)z2[i]-48; if((h1+h2+ueber)>=10) { teilErg = (h1+h2+ueber)-10; erg[i+1] = (char) (teilErg+48); ueber = 1; } else { teilErg = (h1+h2+ueber); erg[i+1] = (char) (teilErg+48); ueber = 0; } } erg[0] = (char) (ueber+48); } void ausgabe(char erg, int ergLaenge) { int i = 0; for(i; i<(ergLaenge+1); i++) { putchar(erg[i]); } } int laengeBestimmen(char array, int arrayLaenge) { int schleife = 1; int laenge = 0; int zaehler = 0; while(schleife) { if(array[zaehler] == 10) schleife = 0; else { laenge++; } if(zaehler < arrayLaenge) { zaehler++; } else { schleife = 0; } } return laenge; } void arrayInhaltVerschieben(char array,int arrayLimit, int verschieben) { char h1, h2; int i = 0; for(i; i<verschieben; i++) { int j = 0; h2 = array[j]; for(j = 0; j<arrayLimit-1; j++) { h1 = h2; h2 = array[j+1]; array[j+1] = h1; } array[i] = 10; } }

Ich danke euch schon mal für eure Zeit und eure Antworten.

mfg

Unregistrierter

Nach dem ersten drüberschauen:
Bei 48 meinst du doch sicherlich '0'. Dann schreib auch '0'.
Damit kannst du auch rechnen.

Du hast LIMIT definiert, dann nutze es auch bei fgets.
Ach ja, fgets liest das '\n' mit ein. Musst du da nicht zweimal Enter drücken, wenn die Zahl kurz ist?

Was soll eigentlich das hier:

C++:
if(erg[i] == '0') printf("0"); //wenn nullen in der Zahl existieren if(erg[i] != '0') { printf("%c", erg[i]);

Da reicht ein

C++:
putchar(erg[i]);

Ich verstehe dass //-NEWLINE- ans Arrayende setzen und vorige mit '0' auffüllen nicht.

Ich hätte die Strings einfach umgedreht, so dass die Einerstelle am Index 0 steht.
Dann addiert und wieder umgedreht.
Dann ganz normal mit puts(erg); oder printf("%s", erg ); ausgegeben.

Unregistrierter

DirkB schrieb:

Ich verstehe dass //-NEWLINE- ans Arrayende setzen und vorige mit '0' auffüllen nicht.

Nehme ich zurück.

Aber

C++:
zahl1[LIMIT] = 10; zahl2[LIMIT] = 10;

geht nicht, da das Element mit dem Index LIMIT gar nicht existiert.
Da meinst du sicherlich:

C++:
zahl1[LIMIT-1] = '\n'; zahl2[LIMIT-1] = '\n';

Ob du sonst noch an den Indexgrenzen Probleme hast z.B.:
for(i; i<(ergLaenge+1); i++) { seh ich gerade nicht.

rufrider Mitglied 22:24:11 09.06.2012 Titel:

Danke für die Antworten,

C++:
printf("%s", erg );

und

C++:
zahl1[LIMIT-1] = '\n'; zahl2[LIMIT-1] = '\n';

hab ich mal Eingefügt.

Die grenze bei for(i; i<(ergLaenge+1); i++) stimmt so.

Wutz Mitglied 09:06:10 10.06.2012 Titel:

- gut ist, dass du Funktionen verwendest um Funktionalität zu kapseln
- schlecht ist, dass du ohne Not C99 verwendest
- insgesamt zuviel Code, laengeBestimmen/arrayInhaltVerschieben sind überflüssig wie auch Parameter ergLaenge
- du prüfst nicht auf sinnvolle Nutzereingaben
- du verwendest 10 statt '\n' und nicht einheitlich
- du verwendest 48 statt '0' und nicht einheitlich
- deine Casts sind überflüssig
- deine memset-Initialisierungen sind sinnfrei, weil danach der Speicherbereich sowieso wieder überschrieben wird
- deine Aufgabenstellung ist unpräzise (was ist mit führendem +/-, was ist mit Kommazahlen, ...)
- ...

Unregistrierter

rufrider schrieb:

Danke für die Antworten,

C++:
printf("%s", erg );

Das kannst du in deinem Programm so nicht benutzen, denn es fehlt das Stringendezeichen (das '\0') in erg.

rufrider Mitglied 22:08:04 10.06.2012 Titel:

@Wutz:

Code:
-schlecht ist, dass du ohne Not C99 verwendest

Der C99 benutze ich soviel ich weiß nicht. Ich Programmiere unter
Netbeans und da muss ich bei den Eigenschaften dem Compiler als
Parameter mitgeben, was ich nicht tue.

Code:
insgesamt zuviel Code, laengeBestimmen/arrayInhaltVerschieben sind überflüssig wie auch Parameter ergLaenge

Zu den Funktionen: Ich wusste nicht wie ich es anders Lösse, da ja die Zahlen
unterschiedlich Längen haben können und ohne die Verschiebung die Addition nicht
mehr stimmt.

Code:
du prüfst nicht auf sinnvolle Nutzereingaben

Wäre es die klügste Lösung zu überprüfen ob nur Ziffern von 0-9 (48 - 57[ASCII])
im Array stehen ?

Code:
du verwendest 10 statt '\n' und nicht einheitlich du verwendest 48 statt '0' und nicht einheitlich

Werde ich mir merken.

Code:
deine Casts sind überflüssig

Ich weiß gerade leider nicht was du meinst.

Code:
deine memset-Initialisierungen sind sinnfrei, weil danach der Speicherbereich sowieso wieder überschrieben wird

Das mach ich da im Array ja sonst was stehen könnte und das würde die
Addition beeinflussen. Hoffe ich liege da nicht falsch ?

Code:
deine Aufgabenstellung ist unpräzise (was ist mit führendem +/-, was ist mit Kommazahlen, ...)

Da kann ich mal nix für

. Ich werde es meinem Prof. mal ausrichten.

@DirkB

Code:
Das kannst du in deinem Programm so nicht benutzen, denn es fehlt das Stringendezeichen (das '\0') in erg.

Habe ich Später auch gemerkt. Habe es wieder zu

C++:
for(i; i<(ergLaenge+1); i++) { putchar(erg[i]); }

geändert.

mfg

Unregistrierter

C99 nutzt du schon dann, wenn du miiten im Code Variablen definierst.
Zeile 48 und 81.
Da C99 der aktuelle Standard ist (C11 wird noch nicht so richtig unterstützt, bzw gibt es auch erst seit Dezember 2011), wird es auch die Voreinstellung deines Compilers sein.

rufrider schrieb:

Wäre es die klügste Lösung zu überprüfen ob nur Ziffern von 0-9 (48 - 57[ASCII])
im Array stehen ?

rufrider schrieb:

Wutz schrieb:

du verwendest 10 statt '\n' und nicht einheitlich
du verwendest 48 statt '0' und nicht einheitlich

Werde ich mir merken.

Merkst du etwas?
Vergiss ASCII.
Nimm '0' und '9' bzw. gleich isdigit() aus ctype.h

rufrider schrieb:

Habe ich Später auch gemerkt. Habe es wieder zu
...
geändert.

Du solltest dir unbedingt nochmal Strings in C anschauen.

rufrider Mitglied 22:27:22 11.06.2012 Titel:

Angucken werde ich mir wohl nochmal alles, nicht nur die Strings. Da in
3 Wochen die Klausuren sind.

Wegen der Aufgabe habe ich heute nochmal mit meinem Prof. geredet.
So hat er mir gesagt, dass ich den String auch in ein int array umwandeln darf,
des weiteren muss ich nicht auf Gültige eingaben Prüfen ,es zu speicher overflows kommt oder ähnlichem.
Es soll einfach nur um die Handhabung mit Arrays gehen.

Somit habe ich mich Heute nochmal an eine andere Lösung gesetzt.

C++:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88	#include <stdio.h> #include <stdlib.h> #include <string.h> #define LIMIT 100 /* * / void benutzerEingabe(char z1, char z2); void stringToInt(char string, int zahl); void addieren(int z1, int z2, int erg); void IntToString(int zahl, char string); int main(int argc, char** argv) { char zahlString1[LIMIT+2]; //+2 wegen '\n' und '\0' char zahlString2[LIMIT+2]; //+2 wegen '\n' und '\0' char ergString[LIMIT+2]; //+2 wegen Uebertrag und '\0' int zahl1[LIMIT] = {'0'}; int zahl2[LIMIT] = {'0'}; int erg[LIMIT+1] = {'0'}; //+1 wegen Uebertrag benutzerEingabe(zahlString1, zahlString2); stringToInt(zahlString1, zahl1); stringToInt(zahlString2, zahl2); addieren(zahl1, zahl2, erg); IntToString(erg, ergString); printf("\nErgebnis: %s", ergString); return (EXIT_SUCCESS); } void benutzerEingabe(char z1, char z2) { printf("Addition von zwei langen(%d ziffern) Zahlen.", LIMIT); printf("\n============================================="); printf("\nZahl eins eingeben: "); fgets(z1, LIMIT+1, stdin); while(getchar() != '\n'); //Puffer leeren, wenn Zahl zu groß printf("Zahl zwei eingeben: "); fgets(z2, LIMIT+1, stdin); while(getchar() != '\n'); //Pufffer leeren, wenn Zahl zu groß } void stringToInt(char string, int zahl) { int i, j; for(i=0, j=strlen(string)-2; i<strlen(string)-1; i++, j--) { zahl[i] = string[j]-'0'; } } void addieren(int z1, int z2, int erg) { int i, u = 0; for(i=0; i<LIMIT; i++) { erg[i] = z1[i] + z2[i] + u; if(erg[i] >= 10) { u = 1; erg[i] -= 10; } else { u = 0; } } erg[LIMIT] = u; } void IntToString(int zahl, char *string) { int i, j, k; for(i=LIMIT; i>0; i--) { if(zahl[i] != 0) break; } for(k=i, j=0; k>=0; k--, j++) { string[j] = zahl[k]+'0'; } string[j] = '\0'; }

Ich hoffe das ich ein paar der Hier gegebenen Ratschläge befolgt habe.

mfg

Wutz Mitglied 19:20:01 12.06.2012 Titel:

rufrider schrieb:

C++:
int zahl1[LIMIT] = {};

Wo hast du denn das jetzt wieder her?
Die C-Syntax verlangt eindeutig mind. 1 Element in einer Initialisierer-Liste.

Ich würde das Problem so lösen:

C++:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46	#include <stdio.h> #include <string.h> #include <stdlib.h> #include <assert.h> #define MAXSTELLEN 100 #define Str(x) #x #define Xstr(x) Str(x) /* liest eine Ziffernfolge mit max. MAXSTELLEN Stellen ein, Programmabbruch wenn fehlerhaft / void liesZahlstringOderAbbruch(char s,int n) { assert( MAXSTELLEN>=n ); /* hier gibt es gleich was auf die Finger, wenn der Programmierer schlampt / printf("Geben Sie eine max. %d-stellige Ziffernfolge ein: ",n); if( 1!=scanf("%" Xstr(MAXSTELLEN) "[0-9]",s) \|\| getchar()!='\n' ) fputs("fehlerhafte Eingabe",stderr),exit(1); } / addiert 2 Zahlstrings zu einem Ergebnisstring und gibt Zeiger diesen zurück / const char addiere(const char a,const char b,char c) { int uebertrag=0; const char x=a+strlen(a); const char y=b+strlen(b); c+=MAXSTELLEN+1; while( x!=a \|\| y!=b ) { int d = (x!=a?--x-'0':0) + (y!=b?--y-'0':0) + uebertrag; --c = '0' + d % 10; uebertrag = d > 9; } if( uebertrag ) *--c = '1'; return c; } int main(void) { char a[MAXSTELLEN+1],b[MAXSTELLEN+1],c[MAXSTELLEN+2]=""; liesZahlstringOderAbbruch(a,MAXSTELLEN); liesZahlstringOderAbbruch(b,MAXSTELLEN); printf("Ergebnis: %s\n",addiere(a,b,c)); return 0; }

Alle o.g. Probleme sind hier beseitigt, was würdest du dir selbst für eine Note geben, wenn mein Programm als Referenz eine 1 wäre?