3. Klassen in C++ Durchschnitt von zwei Zahlen berechnen

Klassen in C++ Durchschnitt von zwei Zahlen berechnen

  • J

    @john-0 sagte in Klassen in C++ Durchschnitt von zwei Zahlen berechnen:

    länger im Forum engagieren...Glaubst du ernsthaft normale Menschen haben Lust auf diese fortgesetzte verbalen Entgleisungen(1) der Mods in diesem Forum?

    Ich bin seit 2009 registriert und war davor schon ein paar Jahre nicht registriert unterwegs (ging damals noch). Ich kann mich an keine nenneswerte Entgleisung eines Mods erinnern.

    @john-0 sagte in Klassen in C++ Durchschnitt von zwei Zahlen berechnen:

    Wenn du mich nicht im Forum haben willst, schreib das einfach, anstatt mich jedesmal anzupöbeln.

    Wenn dich jemand nicht im Forum haben wollte, dann hätte sich @SeppJ wohl nicht extra die Mühe gemacht, mal zu schreiben, warum deine Antworten soooo anstrengend sind.

    0

  • Was bin ich froh dass ich nur die Schatten dieser Diskussion sehe 🙂

    0

  • Mich juckt das auch nicht. Habe zwar niemanden ignoriert, aber ich finde es gut, wenn es auch solche Diskussionen gibt. Je nach Kenntnis kann jeder sein eigenes Urteil bilden. Sehe hier auch nicht die Gefahr, das "Unwissende" gefährdet sein könnten.

    0 1 Antwort

  • @john-0 sagte in Klassen in C++ Durchschnitt von zwei Zahlen berechnen:

    @SeppJ sagte in Klassen in C++ Durchschnitt von zwei Zahlen berechnen:

    sondern dass das absolut gar nichts mit dem Thema zu tun hat.

    Es geht um das Thema: Wie beeinflusst protected das Design von Klassen in C++. Das Design von C++ Klassen wird eben nicht nur von der API sondern auch von der ABI beeinflusst.

    Nein. Es geht hier darum, der Threaderstellerin zuerst die wichtigen Ratschläge zu geben. Und da gilt für, mich dass die ABI in diesem Stadium des Lernens gar keine Bedeutung hat und die API eine große Bedeutung hat. In einem anderen Leserkreis wäre deine Diskussion sicherlich relevant, aber in diesem Kontext ist sie nicht zielführend, da sie unter Umständen auch zur Verwirrung beiträgt.

    1

  • @zeropage sagte in Klassen in C++ Durchschnitt von zwei Zahlen berechnen:

    Mich juckt das auch nicht. Habe zwar niemanden ignoriert, aber ich finde es gut, wenn es auch solche Diskussionen gibt. Je nach Kenntnis kann jeder sein eigenes Urteil bilden. Sehe hier auch nicht die Gefahr, das "Unwissende" gefährdet sein könnten.

    Das Urteil, das ich mir aus diesen Diskussionen gebildet habe, ist menschlich, nicht fachlich. Und SeppJ hat es schon ziemlich gut beschrieben.
    Das sind so Diskussionen, auf die ich nicht wirklich Bock habe, weil sie zu 99,99% Dinge behandeln, die ich in meinen Quelltexten nicht habe. Und ich habe genug andere Dinge zu tun, um mich akademische Nischenprobleme zu kümmern.

    0
  • E
    Gesperrt

    Hallo, damit die Streitigkeiten mal ein Ende finden, ich wäre die Sache ungefähr so angegangen:

    #include <iostream>

using namespace std;

class Average
{
private:
    const bool exact;
    int32_t count = 0;
    int64_t sum = 0;
    double avg = 0;

public:
    Average();
    Average(int32_t size);
    ~Average();
    void addElement(int32_t e);
    double getAvg();
};

Average::Average() : exact(true)
{
}

Average::Average(int32_t size) : exact(false), count(size)
{
}

Average::~Average()
{
}

void Average::addElement(int32_t e)
{
    if (exact)
    {
        count++;
        sum += e;
    }
    else
    {
        avg += e / (double)count;
    }
}

double Average::getAvg()
{
    if (exact)
    {
        return sum / (double)count;
    }
    else
    {
        return avg;
    }
}

int main(int argc, char const *argv[])
{
    Average a1;
    Average a2(3 * 10);
    int32_t vals[] = {
        -23, 456, -543};
    for (size_t i = 0; i < 10; i++)
    {
        for (const auto &e : vals)
        {
            a1.addElement(e);
            a2.addElement(e);
        }
    }
    cout.precision(17);
    cout << a1.getAvg() << "\n";
    cout << a2.getAvg() << "\n";
    double expected = -36.666666666666666;
    cout << expected << "\n";
    cout << abs(a1.getAvg() - expected) << "\n";
    cout << abs(a2.getAvg() - expected) << "\n";
    return 0;
}

    Ich denke, das ist so richtig... Die Ausgabe ist bei mir (bei euch hoffentlich auch...):

    -36.666666666666664
-36.666666666666686
-36.666666666666664
0
2.1316282072803006e-14

    (Falls ich was übersehen hab, bitte Hinweise geben 🙂 Danke)

    0

  • Mein Senf dazu:

    1. using namespace ... gehört nicht in Header Dateien
    2. fehlende const-correctness
    3. Warum keine Default-Initialisierung für exact?
    4. Warum ist exact const? Damit verhinderst du, dass Kopien erzeugt werden können, ist das so gewollt? Es ist privat und kann nur über den Konstruktor gesetzt werden, warum machst du das noch zusätzlich const?
    5. C-style casts (Zeile 50) in C++ sind meh
    0
  • E
    Gesperrt

    Danke fürs Feedback/Review @DocShoe ... 🙂

    Warum ich das mache? Weil ich es nicht besser wusste... Hast du einen Änderungsvorschlag? lg.

    0
    S
     1 Antwort
  • S

    @EinNutzer0 Naja, schreibt @DocShoe doch. im Header kein using namespace sondern std:: vor die jeweiligen Sachen schreiben. Wobei du das nur in der main() hast, daher brauchst du im Header gar kein using namespace
    Die Member Funktionen, die const sein können, sollten das auch sein. Das betrifft vor allem deine Getter.
    Anstelle c-Style casts gäbe es auch C++ Alternative.

    Apropos c-Style Cast, noch ein paar Anmerkungen von mir:
    Vielleicht kannst du dein Programm auch so ändern, das gar nicht gecasted werden muss?
    Warum nimmt dein Rechner nur int entgegen? Kann es nicht sein, dass man auch einen Mittelwert von nicht ganzen Zahlen berechnen möchte?

    Was soll die Konvention mit dem exact? Wenn das false ist und die Anzahl der hinzugefügten Elemente nicht mit count übereinstimmt, ist der Mittelwert einfach falsch. Warum bietest du das überhaupt an, das als API ist maximal unintuitiv.

    Was soll der Konstruktor Parameter size? Von was ist das die size? Der Member heißt doch auch anders.

    Warum hast du einen Destruktor definiert?

    In deiner main iterierst du ja direkt über eine range und fügst die Elemente einzeln hinzu. Als Idee für eine Weiterentwicklung, vlt wäre es ja Sinnvoll, wenn dein Rechner direkt auf einer range arbeiten könnte.

    Und zum Schluss: Deine Übrprüfung in der main ist für Nachvolllziehbarkeit im Forum gut, aber für dich wäre es noch ein bisschen besser, wenn du das in Unit Tests ausgliedern würdest 😉

    1
  • Mod

    Unter der Annahme (der Code ist wegen der schlechten Bezeichner recht unlesbar, daher nur Annahme), dass der exact-Parameter irgendwie zwischen zwei unterschiedlichen Strategien wählen soll: Nicht so. if-Abfragen, um zwischen verschiedenen Strategien wählen zu können haben viele Nachteile:

    • Muss an vielen Stellen konsistent gehalten werden -> Katastrophe buchstäblich vorprogrammiert
    • Nicht erweiterbar, wenn mehr Möglichkeiten gebraucht werden
    • Man schleppt überall den Müll von den nicht-benutzten Strategien herum (hier z.B. avg)
    • Laufzeitkosten
    • Schlecht lesbar, da jede einzelne Funktion alle Strategien enthält, und daher jede Funktion aufgeblasen ist und der Code für die verschiedenen Strategien immer abwechselnd durchmischt ist (hier gehören z.B. Zeile 54 und 42 zusammen, dazwischen ist aber Codezeile 50, die zu 37f. gehört)

    Ansonsten schreit der Numeriker in mir, dass exact einen weniger genauen Algorithmus benutzt.

    0
  • E
    Gesperrt

    Ja, ich verfolgte damit zwei unterschiedliche "Summierungsstrategien"...

    Ich werde das noch mal komplett überarbeiten und melde mich später nochmal. 🙂 Hohe Fehleranfälligkeit, nicht erweiterbar und unnötige Laufzeitanforderung leuchtet mir sofort ein.

    0
  • E
    Gesperrt

    Bitte nicht hauen 🙃 , aber habe ich jetzt noch etwas übersehen?

    #include <iostream>
#include <vector>
#include <algorithm>

class Average
{
private:
public:
    virtual void addElement(double e) = 0;
    virtual double getAvg() = 0;
};

class ExactAverage : public Average
{
private:
    uint32_t count = 0;
    double sum = 0;

public:
    ExactAverage(){};
    void addElement(double e)
    {
        count++;
        sum += e;
    }
    double getAvg()
    {
        return sum / count;
    }
};
class NormalAverage : public Average
{
private:
    const uint32_t numberOfElements;
    double avg = 0;

public:
    NormalAverage(uint32_t numberOfElements) : numberOfElements(numberOfElements){};
    void addElement(double e)
    {
        avg += e / numberOfElements;
    }
    double getAvg()
    {
        return avg;
    }
};

int main(int argc, char const *argv[])
{
    const uint32_t n = 3 * 10;
    std::vector<Average *> myAverages;
    myAverages.push_back(new ExactAverage());
    myAverages.push_back(new NormalAverage(n));
    double vals[] = {
        -23.45, 456.7, -543.21};
    for (size_t i = 0; i < 10; i++)
    {
        for (size_t j = 0; j < sizeof(vals) / sizeof(vals[0]); j++)
        {
            for_each(myAverages.begin(), myAverages.end(), [&](Average *a)
                     { a->addElement(vals[j]); });
        }
    }
    std::cout.precision(17);
    for_each(myAverages.begin(), myAverages.end(), [&](Average *a)
             { std::cout << a->getAvg() << "\n"; });
    return 0;
}

    Das new geht nicht, oder?

    0 1 Antwort
  • Mod

    @EinNutzer0 sagte in Klassen in C++ Durchschnitt von zwei Zahlen berechnen:

    habe ich jetzt noch etwas übersehen?

    Fettes Speicherleck, und das merkwürdige Design von ExactAverage, das wohl jedem hier ein komplettes Rätsel ist.

    0
  • E
    Gesperrt

    @SeppJ Wie geht es besser (das Design)? Abstrakte Klassen dürfen ja keine Member haben...

    0 2 Antworten

  • @EinNutzer0

    Warum nicht so?

    #include <numeric>
#include <vector>
#include <cmath>
#include <iostream>


class Average
{
private:
    size_t mCount = 0;
    double mSum = 0;

public:
    void Add(double v)
    {
        mSum += v;
        mCount++;
    }
    
    double GetAvg() const
    {
        return mSum / mCount;
    }
    
    //double GetStdDev() const ...
};

class MovingAverage
{
private:
    std::vector<double> mValues;
    size_t mSize;                       ///< Fenstergröße
    size_t mWritePos = 0;

public:
    explicit
    MovingAverage(size_t Size = 5) : 
        mSize(Size)
    {    
    }
    
    void Add(double v)
    {
        if (mValues.size() < mSize)
        {
            mValues.push_back(v);
        }
        // Haben wir bereits mSize Elemente eingelesen, so überschreiben 
        // wir den ältesten Wert.
        else
        {
            mValues[mWritePos] = v;
            mWritePos = (mWritePos + 1) % mSize;
        }
    }
    
    double GetAvg() const
    {
        return std::accumulate(std::begin(mValues), std::end(mValues), 0.0) / mValues.size();
    }
    
    //double GetStdDev() const ...
};


int main(void) {
    constexpr double Tolerance = 1E-6;
    const std::vector<double> Values {1.54, 2.12, 3.23, 4.62, 5.91, 6.13, 7.02, 8.34, 9.83};
    Average a;
    MovingAverage a2;
        
    std::for_each(std::begin(Values), std::end(Values), [&](double v) {
        a.Add(v);
        a2.Add(v);
    });     

    if (std::fdim(a.GetAvg(), 5.415555) < Tolerance)
        std::cout << "Test mit Average war erfolgreich";
    else
        std::cout << "Test mit Average war nicht erfolgreich";
    
    if (std::fdim(a2.GetAvg(), 7.446) < Tolerance)
        std::cout << "Test mit Average2 war erfolgreich";
    else
        std::cout << "Test mit Average2 war nicht erfolgreich";
    return 0;
}
    0
  • W

    Ich halte das für komplett overdone. Klassenhierachie für eine Mittelwert-Berechnung? Ernsthaft?

    Ich wäre da in eine ganz andere Richtung gegangen und hätte möglicherweise den Typ für die Summe getemplatet. Normalerweise sollte man dann einfach ein Average<double> wollen, aber vielleicht braucht man ja mal einen anderen Typ? Und ganz ehrlich: wenn man die Anzahl Elemente vorher kennt, dann würde ich eher einfach eine Summe bilden wollen und von Hand teilen als einem Average-Objekt das vorher mitzuteilen.

    Man kann auch einfach von boost ein accumulator_set<double, stats<tag::mean>> verwenden - bei mean ist es den boost-Aufwand nicht wirklich wert, aber wenn man noch mehr über die Zahlen wissen will, die man so hat, dann könnte es lohnen 😉 Auch den Moving Average von @Quiche-Lorraine kann man damit gleich erschlagen. Siehe https://www.boost.org/doc/libs/1_80_0/doc/html/accumulators/user_s_guide.html

    0 1 Antwort

  • @EinNutzer0 sagte in Klassen in C++ Durchschnitt von zwei Zahlen berechnen:

    @SeppJ Wie geht es besser (das Design)? Abstrakte Klassen dürfen ja keine Member haben...

    Doch, dürfen sie. Ich frage mich aber, warum du überhaupt eine Klassenhierachie baust. Sollen die einzelnen Objekte/Strategien austauschbar sein? Wenn du verallgemeinern willst, dann hol´ doch noch weiter aus und nimm´ sowas als Basisklasse:

    #include <cmath>
#include <memory>
#include <vector>

template<typename T>
class UnaryNumericOperation
{
public:
   UnaryNumericOperation() = default;
   virtual ~UnaryNumericOperation() = default;

   virtual T    result() const = 0;
   virtual void update( T value ) = 0;
};

class AverageCalculator ; public UnaryNumericOperation<double>
{
   double Total_ = 0;
   unsigned int Count_ = 0;
public:
   AverageCalculator() = default;

   double result() const override
   {
      if( Count_ == 0 ) return std::nan( "" );
      else              return Total_ / Count_;
   }

   void update( double value ) override
   {
      Total_ += value;
      ++Count_;
   }
};

int main()
{
   using UnaryOpsPtr_t = std::unique_ptr<UnaryNumericOperation>;
   std::vector<UnaryOpsPtr_t> ops;
   ops.push_back( std::make_unique<AverageCalculator>() );

   ...
}

    Je nach Geschmack kannste die Basis- oder abgeleitete Klassen als template/konkrete Klasse implementieren, dann musste dir nur Gedanken darüber machen, wie ungültige Ergebnisse behandelt werden sollen (Ergebnis als std::optional zB).

    0

  • @wob sagte in Klassen in C++ Durchschnitt von zwei Zahlen berechnen:

    Ich halte das für komplett overdone. Klassenhierachie für eine Mittelwert-Berechnung? Ernsthaft?

    Hihi, wir benutzen genau sowas. Wir haben Messdatenreihen, für die sich der Kunde passende Filter zusammenklicken kann, um sich sein Diagramm zusammenzustellen.

    0
    W
     2 Antworten
  • W

    @DocShoe sagte in Klassen in C++ Durchschnitt von zwei Zahlen berechnen:

    Hihi, wir benutzen genau sowas.

    Vermutlich nicht, wenn ich dich richtig verstehe. Ihr habt (vermute ich) eine Klassenhierachie für verschiedene Werte, die man berechnen kann. Nicht verschiedene Möglichkeiten, den Mittelwert zu berechnen. Also eher: mach irgendwas mit einer Zahlenfolge. -> Variante 1: Mittelwert. Variante 2: Summe. Oder so. Aber doch nicht x verschiedene Varianten für ein und denselben Wert, die ihrerseits wieder Struktur haben.

    Und mir ist klar, dass man abhängig von der Datenmenge manchmal z.B. aufsummierende Varianten haben will, die nicht so exakt arbeiten wie andere (z.B. wenn man die Varianz berechnen will). Aber auch da wäre dann die Frage, ob die wirklich voneinander erben sollten (oder eine abc haben sollen). Beide könnten auch bei dir von der globalen "mach was mit Zahlen"-Klasse erben.

    Generell finde ich kleinere / flachere Klassenhierachien immer besser.

    0
  • Mod

    @wob sagte in Klassen in C++ Durchschnitt von zwei Zahlen berechnen:

    Ich halte das für komplett overdone. Klassenhierachie für eine Mittelwert-Berechnung? Ernsthaft?

    Ich dachte es ginge um eine Demonstration, wie man so etwas stringent designt. Ob in den Funktionen, die tatsächlich etwas tun, dann ein trivialer Einzeiler oder wer weiß was für komplexe Berechnungen stattfinden, ist ja egal für die Fragestellung, wie man beispielsweise unterschiedliche Strategien für diese Berechnungen sauber implementieren kann.

    Um meinen Senf dazu zu geben: Ich hätte hier die Strategien vermutlich als ein Plugin-System auf Templateebene implementiert. So a la AverageInterface<MovingAverageEngine> und AverageInterface<AccumulatingAverageEngine>. Denn wie auch schon wob und DocShoe sehe ich keinen Grund, wieso sich dies polymorph verhalten sollte. Man wird ja stets nur eine Strategie für eine Aufgabe wählen.

    @EinNutzer0 sagte in Klassen in C++ Durchschnitt von zwei Zahlen berechnen:

    @SeppJ Wie geht es besser (das Design)? Abstrakte Klassen dürfen ja keine Member haben...

    Was besser? Weder das Speicherleck noch das komische Interface für deinen ExactAverage haben etwas mit exakten Klassen zu tun. Das erste ist einfach eine Frage der korrekten technischen Umsetzung, wie man korrektes C++ programmiert, und das zweite ist halt einfach nur komisch und keiner hier weiß, wieso du das überhaupt so gemacht hast anstatt wie bei der anderen Variante mitzuzählen.

    0
Anmelden zum Antworten