Was macht man mit solch einer Fehlermeldung?


  • Mod

    Du hast offenbar nicht verstanden, was ich dir sagen möchte, was an deinem Design nicht stimmt. Vermutlich aus irgendeinem falsch verstandenem Ehrgefühl, dass das Aufzeigen einer Schwäche in deinem Code irgendwie als persönlichen Angriff wahr nimmt, anstatt inhaltlich über die Kritik nachzudenken.



  • Nein, das hat nichts mit Ehre zu tun, sondern, das die Kritik ziellos ist. Du sagst, konfus, weil ich einen const-Member habe. Ich sage, das war der Fehler. Was war denn noch, was ich jetzt an Verbesserungsvorschlägen überlesen habe?



  • Ihr müsst bedenken, Ihr arbeitet professionell, mache vielleicht sogar als Profs. Ihr könnt Eure Kenntnisse nicht vorrausetzen bei Leuten, die das nebensächlich aus Interesse betreiben.



  • @zeropage sagte in Was macht man mit solch einer Fehlermeldung?:

    	val_t& left() { return p0_.x(); }
    	val_t& bottom() { return p0_.y(); }
    	val_t& right() { return p_.x(); }
    	val_t& top() { return p_.y(); }
    

    Ich finde das hier recht ungewöhnlich. Generell ist es sehr unüblich (nicht-const-)Referenzen auf private Member rauszugeben, wohingegen const-Referenzen absolut state-of-the-art sind.

    Wenn du private Member ändern willst, solltest du eine Setter dafür anlegen, oder eben irgendeine Funktion die Daten verarbeitet und anhand der Daten indirekte diese privaten Member verändert. Abhängig davon was genau die Aufgabe deiner Klasse ist.
    Wenn die Klasse nur ein Container ist, dann reicht ein Setter, wenn die Klasse aber komplexere Aufgaben übernimmt ( ein Parser z.B. ) dann ist beispielsweise "handleMyData( const MyBigData & )"-Funktion, die dann indirekt zur Veränderung deiner privaten Member führt auch sinnvoll.


  • Mod

    @zeropage sagte in Was macht man mit solch einer Fehlermeldung?:

    Nein, das hat nichts mit Ehre zu tun, sondern, das die Kritik ziellos ist. Du sagst, konfus, weil ich einen const-Member habe. Ich sage, das war der Fehler. Was war denn noch, was ich jetzt an Verbesserungsvorschlägen überlesen habe?

    Der ursprüngliche Fehler war nicht, dass dein Member const ist, sondern eine Referenz. Da du gesagt hast, dass du den Fehler behoben hast, aber nicht wie, ist nicht klar, ob er noch const ist oder nicht. Wenn du jetzt sagst, es war ein Fehler, dass es auch falsch ist, dass er const ist, dann habe ich doch Recht gehabt, dass es entweder falsch ist, dass er const ist, oder an anderer Stelle beschrieben wird.

    Die andere Kritik ist, dass deine Klasse zwei Aufgaben erfüllt: Einerseits bietet sie ein low-level Interface zum Speichern von zwei Punkten an (p, p0), andererseits die High-Level Interpretation als Rechteck mit Ecken und Kanten (left, top, etc.). Die High-Level Abstraktion sollte nicht das Implementierungsdetails nach außen geben, dass das Rechteck intern als zwei Punkte gespeichert ist. Sonst bist du für alle Zeiten an dieses spezielle interne Detail gebunden.
    Vorschlag: Eine Klasse zur Speicherung und Manipulation von zwei Punkten, und eine andere Klasse für reine Rechtecklogik, die die Zweipunktklasse (oder halt auch etwas anderes!) intern benutzt. Wobei die Zweipunktklasse wahrscheinlich keine "richtige" selbstgeschriebene Klasse sein braucht, weil ein typedef auf ein Zwei-Punkte-Array die Funktionalität quasi perfekt abbildet



  • @It0101 sagte in Was macht man mit solch einer Fehlermeldung?

    Ich finde das hier recht ungewöhnlich. Generell ist es sehr unüblich (nicht-const-)Referenzen auf private Member rauszugeben, wohingegen const-Referenzen absolut state-of-the-art sind.

    Ja ok, das sehe ich ein. Ich habe das vielleicht zu sehr verallgemeinert. Aber es gibt doch zB auch den operator() als Referenz. Das ist doch hier nichts anderes.



  • @SeppJ sagte in Was macht man mit solch einer Fehlermeldung?:

    Wobei die Zweipunktklasse wahrscheinlich keine "richtige" selbstgeschriebene Klasse sein braucht, weil ein typedef auf ein >Zwei-Punkte-Array die Funktionalität quasi perfekt abbildet

    Der Point2D-Typ wird mit Matrizen multipliziert, weshalb das so nicht geht. Glaube ich.



  • Ich persönlich tue mich bei so reinen einfachen Datenobjekten immer schwer damit, den Vorteil einer "Full-Grown"-Klasse zu sehen gegenüber

    template<typename T>
    struct Rect {
        T top, left, right, bottom;
    };
    

    Manchmal ist simpler besser. Aber das ist ja eine ganz andere Diskussion.


  • Mod

    @zeropage sagte in Was macht man mit solch einer Fehlermeldung?:

    @It0101 sagte in Was macht man mit solch einer Fehlermeldung?

    Ich finde das hier recht ungewöhnlich. Generell ist es sehr unüblich (nicht-const-)Referenzen auf private Member rauszugeben, wohingegen const-Referenzen absolut state-of-the-art sind.

    Ja ok, das sehe ich ein. Ich habe das vielleicht zu sehr verallgemeinert. Aber es gibt doch zB auch den operator() als Referenz. Das ist doch hier nichts anderes.

    Ich weiß nicht ganz, worauf du dich hier mit dem operator() beziehen möchtest. Was gemeint ist, dass man normalerweise keine privaten Member durch Referenzen nach außen offenbart. Denn dann sind sie nicht wirklich privat. Angenommen du fügst eine Funktion hinzu, die die Fläche zurück gibst. Angenommen, du stellst fest, dass die Rechnung dazu zu lange dauert, und fügst einen weiteren privaten Member area hinzu, der sich das Ergebnis merkt, und die Fläche wird nur dann neu berechnet, wenn sich die Eckpunkte verändern. Wenn jetzt deine Klasse aber freies Beschreiben der privaten Eckpunkte von außen erlaubt, dann hat sie die Kontrolle über die interne Invariante verloren, dass in area die Fläche zu den momentanen Eckpunkten gesetzt ist. Das Verändern der Eckpunkte sollte nur über speziell dafür vorgesehene Funktionen geschehen dürfen (die dann area neu berechnen können), nicht über unkontrollierbare Referenzen.



  • @wob Ich werde natürlich versuchen, die Klasse ein wenig einzudampfen, wenn das soviel Anstoss erregt. Aber nur soweit, wie es mir nützt. Wenn es mir nichts bringt, mache ich es nicht.



  • Was @SeppJ sagen will:
    Es gibt jede Menge Alternativen zur Herausgabe von privaten Membern. Die schlimmste Alternative wäre, die Member public zu machen, die beste wäre "setter" und "getter" zu machen. Sage ich jetzt mal so, ohne die Funktion der Klasse zu kennen.



  • @zeropage sagte in Was macht man mit solch einer Fehlermeldung?:

    @wob Ich werde natürlich versuchen, die Klasse ein wenig einzudampfen, wenn das soviel Anstoss erregt. Aber nur soweit, wie es mir nützt. Wenn es mir nichts bringt, mache ich es nicht.

    Die siehst das falsch. Es geht hier nicht darum ob dein Quelltext "Anstoß" erregt. Das ist uns letztendlich egal. Aber du bekommst hier quasi von allen Seiten eigentlich die gleiche Kritik. In vielen Themen kann man bei C++ durchaus diskutieren, aber gerade bei deinem Thema herrscht relative Einigkeit. Ich kann dir nur raten, dir die Vorschläge zu Herzen zu nehmen. Schließlich willst du ja auch dazu lernen.



  • @SeppJ sagte in Was macht man mit solch einer Fehlermeldung?:

    Ich weiß nicht ganz, worauf du dich hier mit dem operator() beziehen möchtest.

    Gemeint ist, das mit solchen Operatoren auch ein privater Member nach außen geholt wird und als Referenz auch überschrieben wird.


  • Mod

    @zeropage sagte in Was macht man mit solch einer Fehlermeldung?:

    Ich werde natürlich versuchen, die Klasse ein wenig einzudampfen, wenn das soviel Anstoss erregt. Aber nur soweit, wie es mir nützt. Wenn es mir nichts bringt, mache ich es nicht.

    Wir stoßen uns nicht an, und wir wollen dich auch nicht fertig machen. Natürlich bezweifelt niemand, dass du eine Klasse mit vier Membern voll im Griff hast. Aber ich will dir zeigen, wo theoretische Probleme mit deiner Modellierung liegen, selbst wenn sie praktisch bei einem derart einfachen Problem keine Rolle spielen. Denn wo, wenn nicht an solch einfachen Fragestellungen, kann man die Designprobleme verstehen und üben? Denn wenn du eine komplizierte Fragestellung hast, dann bist du hauptsächlich mit der Schwierigkeit der Fragestellung beschäftigt. Wenn du dann gutes Design nicht gewöhnt bist, wirst du die schwierige Fragestellung aus Unerfahrenheit mit einem schlechten Design angehen. Und das hast du dann nicht mehr im Griff.



  • Ganz ehrlich, ich glaube nicht daran, das ich jemals wirklich komplizierte Sachen lösen werde. Dafür fehlt mir eigentlich alles. Es gibt Künstler, die machen Ausstellungen, verdienen Geld, erfinden eine neue Kunstform und es gibt Leute, die pinseln im Privaten ein wenig auf Papier rum. Ich gehöre zu letzteren.



  • Was ich mich jetzt allerdings frage, ob die viele Kritik nur zu diesem Beispiel vorliegt, oder ob generell meine ganze Vorstellung von Code schreiben falsch ist.
    Vielleicht sollt ich einige übersichtliche Klassen von mir zeigen, ob da ein grundsätzliches Problem vorliegt. Denn, auch wenn es nur im Privaten ist, eingermaßen korrekt sollte es schon ein.



  • @wob sagte in Was macht man mit solch einer Fehlermeldung?:

    Ich persönlich tue mich bei so reinen einfachen Datenobjekten immer schwer damit, den Vorteil einer "Full-Grown"-Klasse zu sehen gegenüber

    template<typename T>
    struct Rect {
        T top, left, right, bottom;
    };
    

    Manchmal ist simpler besser. Aber das ist ja eine ganz andere Diskussion.

    Wenn du jetzt noch die Reihenfolge von top und left vertauscht, dann ... 🙂


  • Mod

    @zeropage sagte in Was macht man mit solch einer Fehlermeldung?:

    Was ich mich jetzt allerdings frage, ob die viele Kritik nur zu diesem Beispiel vorliegt, oder ob generell meine ganze Vorstellung von Code schreiben falsch ist.

    Nee, ist schon ganz ok so. Damit kannst du gut zurecht kommen für deine eigenen Programme



  • @zeropage sagte in Was macht man mit solch einer Fehlermeldung?:

    Was ich mich jetzt allerdings frage, ob die viele Kritik nur zu diesem Beispiel vorliegt, oder ob generell meine ganze Vorstellung von Code schreiben falsch ist.
    Vielleicht sollt ich einige übersichtliche Klassen von mir zeigen, ob da ein grundsätzliches Problem vorliegt. Denn, auch wenn es nur im Privaten ist, eingermaßen korrekt sollte es schon ein.

    Die Einstellung gefällt mir schon besser. Du kannst gerne eine paar Klassen-Definitionen posten.
    Du hast ja die Grundlagen drauf, soweit ich sehen kann. Im Grunde müssen bei nur noch ein paar Ecken und Kanten geschliffen werden.
    Was @hustbaer , @SeppJ , @Bashar und andere alte Hasen dir vorschlagen, hat in aller Regel Hand und Fuß. Es macht also Sinn, zumindest mal drüber nachzudenken.

    Hier ein paar Grundregeln ( Checkliste ) beim Design von Klassen:
    https://www.cs.odu.edu/~zeil/cs333/latest/Public/checklist/index.html



  • Ich musste die Klasse Rectein wenig ändern, damit sie auch tatsächlich bei mir passt und ich denke, dadurch fallen einige Kritik-Punkte weg.
    Dazu eine Bemerkung. Ich war ja etwas konfus, über die Bemerkung von SeppJ "konfus". Inzwischen meine ich das zu verstehen. Als Leser hatte man den Eindruck, die ganze Geschichte sei etwas konfus. Und das war sie ja auch, ich war ziemlich verhuscht während dieser Klasse, sonst wären es 1.) leine const-Member geworden und 2.) ist es mir 2 Tage nicht aufgefallen und ich habe wie blöd nach dem Fehler gesucht. Das muss schon etwas sonderbar wirken. Kann aber nicht garantieren, das es jetzt klarer auschaut.

    Die aktuelle Klasse Rect

    template<typename T>
    class Rect
    {
    	using val_t = T;
    public:
    	Rect() = default;
    	Rect( const val_t x0, const val_t y0, const val_t x, const val_t y) 
    		: points_ { Point2D<val_t>(x0, y0), Point2D<val_t>(x, y0) , Point2D<val_t>(x, y) , Point2D<val_t>(x0, y) } {}
    
    	std::vector<Point2D<val_t>> points() const { return points_; } //temporär drin, noch nicht sicher ob ich es auch benötige
    
    	val_t left() const { return points_[0].x(); }
    	val_t bottom() const { return points_[0].y(); }
    	val_t right() const { return points_[2].x(); }
    	val_t top() const { return points_[2].y(); }
    
    private:
    	std::vector<Point2D<val_t>> points_;
    };
    

    Und weil es Spekulationen über den Typ Point2Dgab, hier die entsprechende Klasse. Die betrachte ich als abgeschlossen.

    template<typename T>
    class Point2D
    {
    	using val_t = T;
    	using size_t = std::size_t;
    public:
    	Point2D() = default;
    	Point2D(const val_t x, const val_t y)
    		: point{ Mat::Point2D<val_t>(x, y) } {}
    
    	Point2D(const Mat_T<val_t>& mT)
    		: point{ mT } {}
    
    	Point2D(const std::vector<val_t>& pVec)
    	{
    		if (pVec.size() != 2)
    			throw std::out_of_range("Point2D(vector): vector.size(): "
    				+ std::to_string(pVec.size()));
    
    		point(0, 0) = pVec[0];
    		point(0, 1) = pVec[1];
    		point(0, 2) = 1;
    	}
    
    	Point2D<val_t> operator*(const Mat_T<val_t>& mT) const
    	{
    		Point2D<val_t> p = (*this);
    		multiply(p, mT);
    		return p;
    	}
    
    	val_t operator()(const size_t row, const size_t column) const
    	{
    		if (row > 1 || column > 2)
    			throw std::out_of_range("Point2D::operator(): row: "
    				+ std::to_string(row) + " column: "
    				+ std::to_string(column));
    
    		return point(row, column);
    	}
    
    	val_t& operator()(const size_t row, const size_t column)
    	{
    		if (row > 1 || column > 2)
    			throw std::out_of_range("Point2D::operator(): row: "
    				+ std::to_string(row) + " column: "
    				+ std::to_string(column));
    
    		return point(row, column);
    	}
    
    	val_t x() const { return (*this)(0, 0); }
    	val_t y() const { return (*this)(0, 1); }
    	val_t& x() { return (*this)(0, 0); }
    	val_t& y() { return (*this)(0, 1); }
    
    	void print(const int cwidth = 3, std::ostream& stream = std::cout)
    	{
    		const int cp = 4;
    		const int cpwidth = cwidth + cp;
    		stream << "\nx:" << std::setw(cpwidth) << std::fixed << std::setprecision(cp) << x();
    		stream << "  y:" << std::setw(cpwidth) << std::fixed << std::setprecision(cp) << y();
    	}
    
    private:
    	Mat_T<val_t> point = Mat::Point2D<val_t>(0, 0); //eine 1x3 Matrix, zu verfolgen unter 'Entwicklung einer einfachen Matrizenklasse'
    
    	void multiply(Point2D& p, const Mat_T<val_t>& mT) const
    	{
    		Mat_T<val_t> pMat = Mat::Point2D<val_t>(p(0, 0), p(0, 1)) * mT;
    		p = pMat;
    	}
    };
    

Anmelden zum Antworten