Längstes Word in einem Array



  • Hallo Leute,
    Ich versuche ein Programm zu schreiben, dass mir das Längste Wort in einem Array ausgibt. Leider bekomm ich immer nur das erste Wort zurück und ich weiß nicht wieso.

    Folgend mein Code:

    #include <iostream>
    #include <cstring>
    
    using namespace std;
    
    void longestword(char * str){
    
        int wordlen = 0;                            //länge des längsten Wortes
        char word[4000];                            //Char Array für das längste Wort
        int firstchar = 0;                          //Position des ersten Buchstaben des Länsgten Wortes
        int counter = 0;                            //Zählt die Buchstaben eines Wortes
    
        for (int i = 0; i < strlen(str); i++) {
    
            if (str[i] != 32) {                     //32 = Ascii Code für ' ' Space
                counter++;
            }
    
            else if(counter > wordlen){             
                    wordlen = counter;
                    counter = 0;
                    firstchar = i - wordlen;
                }
    
        }
    
        for (int i = 0; i < wordlen; i++) {
            word[i] = str[i + firstchar];
        }
    
        cout << word;
    
    }
    
    int main(){
    
        int size = 4000;
    
        char newstr[size];
        char * str[size];
        *str = newstr;
    
        cout << "Geben sie einen Satz ein:" << endl;
    
        cin.getline(*str, 4000);
    
        longestword(*str);
    
        return 0;
    }
    

  • Mod

    alexxd_12 schrieb:

    Leider bekomm ich immer nur das erste Wort zurück

    Kann ich nicht nachvollziehen.


  • Mod

    Das else-Konstrukt ist falsch. So wird dein Counter ja nur zurück gesetzt, wenn das neue Wort länger ist als das bisherige. Das heißt, mehrere Worte würden als eines gezählt!

    Aber da ist noch mehr, was du ganz dringend ändern solltest:

    • In C++ machst du nichts mit Arrays und noch viel weniger mit nullterminierten Zeichenketten. Das ist C. In C++ würdest du mindestens std::string benutzen. Damit erledigen sich auch automatisch ein paar der weiteren Probleme.

    • strlen als Bedingung einer for-Schleife ist auch in C eine Todsünde. Das geht jedes Mal die ganze Zeichenkette durch, nur um die Länge zu messen! Massivste Verschwendung von Zeit. Mit std::string wär das nicht passiert.

    • Was, wenn das Wort > 4000 Zeichen hat? Katastrophe. Mit std::string wär das nicht passiert.

    • Wozu überhaupt das ganze Wort kopieren? Du kennst schließlich Anfang und Ende. Selbst in C könntest du es damit einfach zeichenweise ausgeben.

    • Wenn du ein Leerzeichen haben willst, warum schreibst du dann magische Zahlen wie 32? Schreib doch ' ' ! Dann funktioniert das auch, wenn das Leerzeichen nicht 32 ist, was nämlich überhaupt nicht garantiert ist, dass das so ist.

    • Möchtest du nicht lieber auf alle Arten von Trennzeichen prüfen? Die Standardbibliothek (sowohl in C als auch in C++) bietet jeweils Methoden dafür an.

    • const-correctness gibt es bei dir nicht. Einerseits gibst du dir mit deiner oben beschriebenen Kopiererei viel Mühe das Original nicht zu verändern (War das deine Motivation?), andererseits machst du das in der Funktionssignatur aber nicht kenntlich. So kann man die Funktion dann unnötigerweise doch nur mit veränderlichen Argumenten aufrufen.

    • Wie du mit Arrays umgehst ist falsch. Was wieder zu der Bemerkung führt, dass man in C++ erst gar keine Arrays benutzen würde. Dein Code ist zwar letztlich funktionierend, aber auch mit C und Arrays könnte man vieles besser machen:

    • Zeile 41 und 42 sind bei dir variable length arrays. Das ist ein obskures C-Feature, welches in seltenen Fällen seine Berechtigung haben mag, aber hier ist es einfach nur unnötig.

    • Zeile 42 ist kein Zeiger auf ein Array, sondern ein Array von Zeigern! Sicherlich nicht, was du möchtest. Wobei ich auch die andere Möglichkeit nicht nachvollziehen kann, wieso du einen Zeiger auf ein Array haben möchtest. Ist die ständige Notwendigkeit in den folgenden Zeilen, überall * dran zu schreiben, kein Alarmzeichen für dich, dass da etwas nicht stimmt?



  • zu 1:

    Sorry, hab wohl vorher den alten Code Kompiliert:

    Wenn ich zum Beispiel: "Leider bekomm ich immer nur das erste Wort zurück" eingebe, bekomm ich als Ausgabe "mer nur das" zurück

    zu 2:

    So haben wir in der Uni leider das Programmieren gelernt. Ich hab bis jetzt kein einziges mal einen String benutzt und wir haben jetzt seit 2 Semestern Programmieren.

    Ich versuch viel zu üben, wir haben aber leider sehr mangelhafte Skripte...



  • Danke für den Input. Habs jetzt zum laufen gebracht, indem ich die Vergleiche Bedienung auch für den Nullterminator geltend gemacht hab.

    hier der (wenn auch nicht schöne) Code

    #include <iostream>
    #include <cstring>
    
    using namespace std;
    
    void longestword(char * str){
    
        int wordlen = 0;                            //länge des längsten Wortes
        char word[4000];                            //Char Array für das längste Wort
        int firstchar = 0;                          //Position des ersten Buchstaben des Länsgten Wortes
        int counter = 0;                            //Zählt die Buchstaben eines Wortes
    
        for (int i = 0; i <= strlen(str); i++) {
    
            if (str[i] != ' ' && str[i] != '\0') {                     //32 = Ascii Code für ' ' Space
                counter++;
            }
    
            else if(counter > wordlen && (str[i] == ' ' || str[i] == '\0')){
                    wordlen = counter;
                    firstchar = i - wordlen;
                }
    
            if (str[i] == ' ') {
                counter = 0;
            }
    
        }
    
        for (int i = 0; i < wordlen; i++) {
            word[i] = str[i + firstchar];
        }
    
        cout << word;
    
    }
    
    int main(){
    
        int size = 4000;
    
        char newstr[size];
        char * str[size];
        *str = newstr;
    
        cout << "Geben sie einen Satz ein:" << endl;
    
        cin.getline(*str, 4000);
    
        longestword(*str);
    
        return 0;
    }
    


  • Ich finde es immer schade, wenn sich jemand die Mühe gibt ausführliches Feedback zu geben, und der Großteil davon, wenn nicht alles wird einfach ignoriert. Wenn Du eigentlich garnicht C++ lernen möchtest, dann ist das hier das falsche Unterforum. Nicht einmal den strlen Fehler hast du behoben; Dir ist bewusst, dass strlen O(n) läuft, d.h. jedes Mal über jeden char im string iteriert, oder?

    Vgl.:

    #include <algorithm>
    #include <iostream>
    #include <string>
    #include <vector>
    
    int main()
    {
        std::vector<std::string> words;
        for (std::string word; std::cin >> word; )
            words.push_back(word);
    
        if (words.empty())
            std::cout << "No Input\n";
        else {
            // könnte man in eine eigene Funktion packen
            auto&& longest = *std::max_element(cbegin(words), cend(words), [](auto&& Lhs, auto&& Rhs) { return Lhs.size() < Rhs.size(); });
            std::cout << "The longest word is: " << longest << '\n';
            // falsch: std::cout << "The longest word is: " << *std::max_element(cbegin(words), cend(words)) << '\n';
        }
    }
    

    P.S.: Mir ist klar, dass man das an der Uni leider nicht lernt, ist bei uns ja auch so. Aber das bedeutet nicht, dass man deshalb nicht wissen darf, wie es besser geht.

    LG



  • HarteWare schrieb:

    *std::max_element(cbegin(words), cend(words))
    

    Das tut nicht, was du denkst. Dazu müsstest du noch ein , [](auto&& Lhs, auto&& Rhs){ return Lhs.size() < Rhs.size(); } anfügen.

    LG



  • Fytch schrieb:

    HarteWare schrieb:

    *std::max_element(cbegin(words), cend(words))
    

    Das tut nicht, was du denkst. Dazu müsstest du noch ein , [](auto&& Lhs, auto&& Rhs){ return Lhs.size() < Rhs.size(); } anfügen.

    LG

    Oh da hast Du aber recht, mein Fehler! Werde es korrigieren, danke. P.S.: Ich muss grad fragen, wieso

    (auto&& Lhs, auto&& Rhs)
    

    sind dass dann rvalue-Referenzen oder Universal-Referenzen?



  • HarteWare schrieb:

    P.S.: Ich muss grad fragen, wieso

    (auto&& Lhs, auto&& Rhs)
    

    sind dass dann rvalue-Referenzen oder Universal-Referenzen?

    Universal-Refs, binden also alle value types und bewahren CV-Qualifier. auto als Lambda-Parameter entspricht template< typename T > void Closure::operator()( T ) , und analog auto&& dem T&& . Anders würde das nicht kompilieren, denn die lvalue-Referenz, die std::vector::const_iterator::operator* zurückgibt, kannst du nicht implizit an eine rvalue-Referenz binden.

    PS: dein auto longest erzeugt eine Kopie des Strings, auch hier wäre ein auto const& oder auto&& angebracht. 😉

    LG



  • OK, danke für die ausführliche Erklärung. Mir war nicht bewusst, dass sich auto&& so verhält 👍



  • @alexxd_12
    Würdest du in deiner for-Schleife die Abbruchbedingung auf < ändern, würde nie das '\0' in der Schleife behandelt werden.
    Somit sind deine Ausnahmen überflüssig.

    for (int i = 0; i < strlen(str); i++) {
    

    Aber um gleich mal die richtige Bedingung zu zeigen:

    for (int i = 0; str[i] != '\0'; i++) { // einfach nur ;str[i]; reicht auch
    

    Was anderes macht strlen auch nicht.

    Und es gilt immer noch, das von SeppJ geschriebene!


  • Mod

    HarteWare schrieb:

    Ich finde es immer schade, wenn sich jemand die Mühe gibt ausführliches Feedback zu geben, und der Großteil davon, wenn nicht alles wird einfach ignoriert.

    Ich auch. Daher gibt es, statt konkreter Hilfe für den Threadersteller, nur eine weitere Stufe der von dir begonnenen Transformation zu einem C++-Einzeiler:

    auto&& longest = 
        *std::max_element(std::istream_iterator<std::string>(std::cin), 
                          std::istream_iterator<std::string>(), 
                          [](auto&& Lhs, auto&& Rhs) { return Lhs.size() < Rhs.size(); });
    

    Keine unnötigen Zwischenvektoren mehr.



  • SeppJ schrieb:

    auto&& longest = 
        *std::max_element(std::istream_iterator<std::string>(std::cin), 
                          std::istream_iterator<std::string>(), 
                          [](auto&& Lhs, auto&& Rhs) { return Lhs.size() < Rhs.size(); });
    

    Das funktioniert so leider nicht, da std::istream_iterator ein Input Iterator ist, std::max_element aber mindestens Forward Iteratoren benötigt, da er einen Iterator in der Mitte der Range zurückgeben kann. Selbst wenn der Code kompiliert, so erzeugt er UB, sofern die Eingabe nicht leer ist, da std::max_element jeweils zwei verschiedene Elemente aus der Range zu vergleichen versucht, was für Input Iteratoren nicht erlaubt ist.
    Ein besserer Einzeiler wäre:

    auto Longest = std::accumulate(std::istream_iterator<std::string>(std::cin),
                                   std::istream_iterator<std::string>(), 
                                   std::string{},
                                   [](auto&& Lhs, auto&& Rhs){ return Lhs.size() > Rhs.size() ? Lhs : Rhs; });
    


  • unnötige[] Zwischenvektoren [...]

    Aber wenn man dann eines Tages noch die Anzahl der Wörter wissen möchte, und vielleicht auch noch das kürzeste Wort, dann sieht es schon wieder anders aus 🙂


  • Mod

    HarteWare schrieb:

    unnötige[] Zwischenvektoren [...]

    Aber wenn man dann eines Tages noch die Anzahl der Wörter wissen möchte, und vielleicht auch noch das kürzeste Wort, dann sieht es schon wieder anders aus 🙂

    Da braucht man eine andere Mitzählfunktion, aber man braucht für keines davon braucht man eine vollständige Liste aller Werte zu speichern.



  • SeppJ schrieb:

    Da braucht man eine andere Mitzählfunktion, aber man braucht für keines davon braucht man eine vollständige Liste aller Werte zu speichern.

    Ok, stimmt auch wieder, so weit habe ich garnicht gedacht.



  • HarteWare schrieb:

    P.S.: Mir ist klar, dass man das an der Uni leider nicht lernt, ist bei uns ja auch so. Aber das bedeutet nicht, dass man deshalb nicht wissen darf, wie es besser geht.

    Da muss ich OP aber ein wenig in Schutz nehmen. Ich hab C++ von einem C Programmierer gelernt und hab ähnlichen Code geschrieben, bevor ich hier ins Forum gefunden habe :D. Man weiß es halt nicht besser und wenn man einen Lehrer hat der sagt, dass das so geht, dann vertraut man da auch ein wenig drauf. Ich schiebe hier den schwarzen Peter deutlich den Fakultäten zu.


Anmelden zum Antworten