4. Code Optimierung/Verbesserung (Addition n-ziffern langer Zahlen)

Code Optimierung/Verbesserung (Addition n-ziffern langer Zahlen)

  • R

    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:

    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:

    #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

    0
  • ?

    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:

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

    Da reicht ein

    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.

    0
  • ?

    DirkB schrieb:

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

    Nehme ich zurück.

    Aber

    zahl1[LIMIT] = 10;
    zahl2[LIMIT] = 10;

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

    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.

    0
  • R

    Danke für die Antworten,

    printf("%s", erg );

    und

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

    hab ich mal Eingefügt.

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

    0

  • - 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, ...)
    - ...

    0
  • ?

    rufrider schrieb:

    Danke für die Antworten,

    printf("%s", erg );

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

    0
  • R

    @Wutz:

    -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.

    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.

    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 ?

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

    Werde ich mir merken.

    deine Casts sind überflüssig

    Ich weiß gerade leider nicht was du meinst.

    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 ?

    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

    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

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

    geändert.

    mfg

    0
  • ?

    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.

    0
  • R

    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.

    #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

    0

  • rufrider schrieb:

    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:

    #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?

    0
  • R
     

    Die C-Syntax verlangt eindeutig mind. 1 Element in einer Initialisierer-Liste.

    Mit dem 1 Element wusste ich leider nicht, habs mal oben verbessert.

    Der Code ist jedenfalls sehr kurz und man benötigt kein umwandeln der Strings
    oder ähnliches. Auch die Überprüfung der Eingabe, sehr nice.

    Nur habe ich mir die assert.h noch nie angeschaut, oder was Xstr(x) macht habe ich gerade keine ahnung. Ich werd mir deine Lösung Morgen (Später) mal genauer
    anschauen.

    Wenn dein Code eine 1 wäre, würde ich meinen wohl mit (3-)4 bewerten.

    0
Anmelden zum Antworten