Ist dieses (einfache) C++ guter Stil?



  • Es soll der Minimalwert, Maximalwert, Anzahl, Summe und Durschnitt der als Konsolen-Parameter übergebenen Zahlenliste ausgegeben werden.
    Die genaue Aufgabenstellung: https://ibb.co/th5Jgxm
    Mein Versuch:

    #include <iostream>
    #include <vector>
    #include <string>
    
    int main(int argc, char **argv)
    {
        if (argc == 2)
        {
            std::string s = argv[1];
            std::vector<int> ints;
            size_t pos = 0;
            std::string token;
            while ((pos = s.find(",")) != std::string::npos)
            {
                token = s.substr(0, pos);
                s.erase(0, pos + 1);
                ints.push_back(std::stoi(token));
            }
            ints.push_back(std::stoi(s));
    
            double min = ints[0], max = ints[0], sum = 0, avg = 0;
            for (size_t i = 0; i < ints.size(); i++)
            {
                int j = ints[j];
                if (j < min)
                    min = j;
                if (j > max)
                    max = j;
                sum += j;
                avg += (double)j / (double)ints.size();
            }
            std::cout << min << ", " << max << ", " << ints.size() << ", " << sum << ", " << avg << std::endl;
        }
        return 0;
    }
    


  • Erstelle eine Anwendung welcher [sic] auf der Kommandozeile beliebig viele Ganzzahlen kommasepariert übergeben werden können. Die Anwendung soll für diese Zahlen dann die folgenden Statstiken berechnen und auf dem Bildschirm ausgeben:


    • Minimalwert
    • Maximalwert
    • Anzahl der übergebenen Zahlen
    • Summe
    • Durchschnitt (Anzeige mit einer Nachkommastelle)

    Beispiele:

    > CalcStats.exe 6,9,15,3,10
    Minimum: 3
    Maximum: 15
    Anzahl: 5
    Summe: 43
    Durchschnitt: 8.6
    
    #include <cstdlib>
    #include <iterator>
    #include <vector>
    #include <string>
    #include <sstream>
    #include <iostream>
    #include <iomanip>
    #include <algorithm>
    #include <numeric>
    
    int main(int argc, char **argv)
    {
        if (argc < 2) {
            std::cerr << "Usage: " << argv[0] << "[Comma-separated list of numbers]\n\n";
            return EXIT_FAILURE;
        }
    
        std::string input;
        for (int i = 1; i < argc; ++i) { // fanservice: commas aren't required, whitespace is sufficient
            input += argv[i];
            input += ' ';
        }
        std::replace(std::begin(input), std::end(input), ',', ' ');
    
        std::istringstream is{ input };
        std::vector<int> values{ std::istream_iterator<int>{ is }, std::istream_iterator<int>{} };
    
        auto min_max { std::minmax_element(std::begin(values), std::end(values)) };
        auto sum     { std::accumulate(std::begin(values), std::end(values), 0) };
        auto average { sum / static_cast<double>(values.size()) };
    
        std::cout <<   "Minimum: " << *min_max.first
                  << "\nMaximum: " << *min_max.second
                  << "\nAnzahl: "  <<  values.size()
                  << "\nSumme: "   <<  sum
                  << "\nDurchschnitt: " << std::fixed << std::setprecision(1) << average << "\n\n";
    }
    

    @EinNutzer0 sagte in Ist dieses (einfache) C++ guter Stil?:

    int j = ints[j];
    

    Kabooom! Durch deinen ständigen Drang das Rad neu zu erfinden hast du nun undefiniertes Verhalten.

    • Auch für den Rest gibt es fertige Lösungen in der Standard Library. Gutes C++? Nein.
    • double für min, max und sum wo int reicht.
    • ints.size() Divisionen sind zu viele wenn eine reicht.
    • C-Style totschlag-casts.
    • (double)j / (double)ints.size() ... der Typ eines Operanden reicht.
    • Das verlangte Ausgabeformat hast du einfach völlig ignoriert.


  • Bei einem Fehler der Verarbeitung ein EXIT_SUCCESS zurück zu liefern ist extrem schräg. Der Rückgabewert ist dafür da einem Shell-Skript zu signalisieren, dass die Verarbeitung erfolgreich war. Wenn die Parameterliste nicht korrekt ist, ist das wohl kaum der Fall.



  • @john-0 Tjoa, stimmt. Geändert.



  • @Swordfish sagte in Ist dieses (einfache) C++ guter Stil?:

    // fanservice: commas aren't required, whitespace is sufficient
    

    Nett. Das nächste Level wäre, bei Bedarf die Zahlen auch von stdin einzulesen und per Flag anzugeben, welche Statistik man gerne hätte (nur die Zahl nach stdout, sonst nix).

    Ich warte noch auf den Tag, an dem man die Aufgaben mal so stellt, dass auch tatsächlich nützliche Tools dabei herauskommen, die man auch in Skripten verwenden könnte. An der Uni hatten wir solche Aufgaben - allerdings in einer fortgeschritteneren Veranstaltung zur Mesh-Verarbeitung (3D-Grafik): Vertexlisten und Dreiecksindizes über stdin rein, verarbeitete Daten über stdout raus und dann via Pipe in ein Anzeigeprogramm - so hats Spass gemacht.

    Geht sicher auch für Anfänger-Aufgaben und hätte sogar den Vorteil, dass man die Studenten-Programme auch automatisiert durch ein Testprogramm jagen kann - "Auto-Korrektur" sollte doch sicher ein paar Dozenten motivieren können 😉



  • @EinNutzer0 sagte in Ist dieses (einfache) C++ guter Stil?:

    int j = ints[j];

    ups, ist mir nicht aufgefallen, das soll natürlich ... ints[i]; sein.

    @Swordfish Bitte an die gegebene Aufgabenstellung halten.



  • @Swordfish sagte in Ist dieses (einfache) C++ guter Stil?:

    @john-0 Tjoa, stimmt. Geändert.

    Jetzt ist es perfekt!



  • @john-0 sagte in Ist dieses (einfache) C++ guter Stil?:

    Jetzt ist es perfekt!

    Nein ist es nicht:

    @Swordfish sagte in Ist dieses (einfache) C++ guter Stil?:

    std::string input;
    for (int i = 1; i < argc; ++i) { // fanservice: commas aren't required, whitespace is sufficient
        input += argv[i];
        input += ' ';
    }
    std::replace(std::begin(input), std::end(input), ',', ' ');


  • @EinNutzer0 Für diese Aufgabe braucht man kein vector.

    Das kann man alles direkt in der Schleife über argv erledigen.



  • Ich hab es so kompiliert bekommen:

    #include <cstdlib>
    #include <iterator>
    #include <vector>
    #include <string>
    #include <sstream>
    #include <iostream>
    #include <iomanip>
    #include <algorithm>
    #include <numeric>
    
    int main(int argc, char **argv)
    {
        if (argc == 2)
        {
            std::string s = argv[1];
            std::replace(std::begin(s), std::end(s), ',', ' ');
            std::istringstream iss(s);
            std::vector<int> values(std::istream_iterator<int>{iss}, std::istream_iterator<int>{});
    
            auto min_max = std::minmax_element(std::begin(values), std::end(values));
            auto sum = std::accumulate(std::begin(values), std::end(values), 0);
            auto average = sum / static_cast<double>(values.size());
    
            std::cout << "Minimum: " << *min_max.first
                      << "\nMaximum: " << *min_max.second
                      << "\nAnzahl: " << values.size()
                      << "\nSumme: " << sum
                      << "\nDurchschnitt: " << std::fixed << std::setprecision(1) << average << "\n\n";
    
            return 0;
        }
        return 1;
    }
    

    { } (Tupel) kannte er nicht...

    @DirkB sagte in Ist dieses (einfache) C++ guter Stil?:

    Das kann man alles direkt in der Schleife über argv erledigen.

    Hab ich mir auch erst gedacht, aber dann würde man nicht alles nutzen, was C++ mitbringt?



  • @EinNutzer0 sagte in Ist dieses (einfache) C++ guter Stil?:

    Hab ich mir auch erst gedacht, aber dann würde man nicht alles nutzen, was C++ mitbringt?

    Ich sehe keinen std::thread in deinem Code.



  • @EinNutzer0 sagte in Ist dieses (einfache) C++ guter Stil?:

    { } (Tupel) kannte er nicht...

    Welcher Compiler auf welcher Plattform?



  • @john-0 sagte in Ist dieses (einfache) C++ guter Stil?:

    Welcher Compiler auf welcher Plattform?

    Neustes Ubuntu und ich compile über Docker: https://hub.docker.com/_/gcc



  • @EinNutzer0 sagte in Ist dieses (einfache) C++ guter Stil?:

    { } (Tupel) kannte er nicht...

    Das ist List Initialization. Seit C++11!

    @EinNutzer0 sagte in Ist dieses (einfache) C++ guter Stil?:

        if (argc == 2)
        {
    

    Preconditions checken und wenn nicht erfüllt nichts wie raus da:

        if (argc != 2)
            return;
    


  • @EinNutzer0 sagte in Ist dieses (einfache) C++ guter Stil?:

    @john-0 sagte in Ist dieses (einfache) C++ guter Stil?:

    Welcher Compiler auf welcher Plattform?

    Neustes Ubuntu und ich compile über Docker: https://hub.docker.com/_/gcc

    Beim g++ lässt sich das Verhalten des Compilers entsprechend der jeweiligen Norm aktivieren in dem man z.B. -std=c++17 angibt. Besser ist es gleich mit g++ -std=c++17 -pedantic -Wall -Wextra zu übersetzen. Bei Bedarf kann man noch weitere Flags setzen, wenn man sie braucht.



  • @john-0 sagte in Ist dieses (einfache) C++ guter Stil?:

    Prüfen auf die betreffende Norm

    Prüfen??



  • @Swordfish sagte in Ist dieses (einfache) C++ guter Stil?:

    @john-0 sagte in Ist dieses (einfache) C++ guter Stil?:

    Prüfen auf die betreffende Norm

    Prüfen??

    Der Compiler verhält sich entsprechend der jeweiligen Norm konform.


Log in to reply