Mengen Klasse



  • Hallo,
    ich habe gerade gemäß Aufgabe in meinem C++ Buch eine Klasse für eine Menge programmiert. Könnte sich das bitte mal jemand anschauen, und sagen, was man noch besser machen kann?

    class.h:

    #ifndef CLASS_H
    #define CLASS_H
    #include <vector>
    
    class IntMenge {
    
    private:
    	std::vector<int> Menge;
    public:
    	void hinzufuegen(int el);
    	void entfernen(int el);
    	bool istMitglied(int el);
    	size_t size();
    	void anzeigen();
    	void loeschen();
    	int getMax();
    	int getMin();
    };
    
    #endif
    

    class.cpp:

    #include"class.h"
    #include<iostream>
    #include<vector>
    using namespace std;
    
    bool IntMenge::istMitglied(int el) {
    	for(size_t i=0; i<(Menge.size()); ++i) {
    		if(Menge.at(i)==el) return true;
    	}
    	return false;
    }
    
    void IntMenge::hinzufuegen(int el) {
    	if(!istMitglied(el)) Menge.push_back(el);
    }
    
    size_t IntMenge::size() {
    	return Menge.size();
    }
    
    int IntMenge::getMin() {
    	if (Menge.size()==0) return 0;
    	int min=Menge.at(0);
    	for(size_t i=1; i<(Menge.size()); ++i) {
    		if(Menge.at(i)<min) min=Menge.at(i);
    	}
    	return min;
    }
    
    int IntMenge::getMax() {
    	if (Menge.size()==0) return 0;
    	int max=Menge.at(0);
    	for(size_t i=1; i<(Menge.size()); ++i) {
    		if(Menge.at(i)>max) max=Menge.at(i);
    	}
    
    	return max;
    }
    
    void IntMenge::anzeigen() {
    	cout << "[";
    	for(size_t i=0; i<(Menge.size()); ++i) {
    		cout << Menge.at(i);
    		if(i!=Menge.size()-1) cout << ",";
    	}
    	cout <<"]" << endl;	
    	cout << "Anzahl: " << Menge.size() << endl;
    	cout << "Min: " << getMin() << endl;
    	cout << "Max: " << getMax() << endl << endl;
    }
    
    void IntMenge::loeschen () {
    	Menge.clear();
    }
    
    void IntMenge::entfernen(int el) {
    	for(size_t i=0; i<(Menge.size()); ++i) {
    		if(Menge.at(i)==el) {
    			Menge.erase(Menge.begin()+i);
    			break;
    		}
    	}
    }
    

    Viele Grüße und besten Dank.



  • Sieht eigentlich gut aus, mir sind nur zwei kleine Sachen aufgefallen:
    Die Dateien immer nach der Klasse bennen, genauso wie die Include Guards.
    In deriner ausgeben() Funktion solltest du die eigene size() Funktion statt Menge.size() verwenden.

    #edit: Achso, prinzipiell lieber kein using namespace verwenden sondern namespaces ausschreiben, kannst es hier aber auch so lassen.



  • Hi,

    deutsch-englisch-gemischte Namen sind sehr unschön.
    '0' kann ein Element der Menge sein. Doof, dass getMin/getMax stumpf 0 zurück geben,
    wenn die Menge leer ist.
    Zum finden der Elemente könntest du auch die fertigen Funktionen aus der vector-Klasse nehmen,
    statt da elber durch zu iterieren.



  • 1.) const correctness

    2.) size_t size();
    genaugenommen liegt size_t im namensraum std. (und benötigt ein include: cstddef )

    3.)

    bool IntMenge::istMitglied(int el) {
        for(size_t i=0; i<(Menge.size()); ++i) {
            if(Menge.at(i)==el) return true;
        }
        return false;
    }
    

    für so etwas würde es std::find geben.
    Wenn du das schon von hand machen möchtest, dann nimm doch bitte den operator[] und nicht at:

    bool IntMenge::istMitglied(int el) {
        for(size_t i=0; i<(Menge.size()); ++i) {
            if(Menge[i]==el) return true;
        }
        return false;
    }
    

    die klammern( i<(Menge.size()) ) sind übrigens sehr verwirrend

    gibt sicher noch andere kleinigkeiten, im großen und ganzen aber nicht schlecht.

    bb



  • vielen dank für das Feedback. Das war meine erste komplett selbst geschriebene Klasse ich bin daher noch recht unerfahren. Ich habe die Dateien class genannt, da ich das Projekt gleich noch für andere Testzwecke verwenden möchte um nicht immer ein neues anlegen zu müssen.
    .at habe ich benutzt, um auf eventuelle Berichsüberschreitung informiert zu werden, da ich eben noch nicht so erfahren bin. Das könnte ich ja noch problemlos durch [] ersetzen.
    Zu find() und sort(). Die STL wurde in dem Buch noch nicht verwendet. Ich habe mir aber mal zum testen eine Methode geschrieben, die meine Menge sortiert:

    void IntMenge::sortieren() {
    	sort( Menge.begin(), Menge.end() );
    }
    

    Die STL geht ja ganz gut zu handhaben.

    Wie könnte ich das denn gestalten, mit min und max bei leerer Menge?

    Und was meinst du mit:
    1.) const correctness ?





  • Mario.. schrieb:

    Wie könnte ich das denn gestalten, mit min und max bei leerer Menge?

    assert



  • statt assert würde ich eher eine exception werfen.
    Assert beendet doch das programm, oder? Während eine Exception dann vom Aufrufer behandelt werden könnte.



  • blub² schrieb:

    #edit: Achso, prinzipiell lieber kein using namespace verwenden sondern namespaces ausschreiben, kannst es hier aber auch so lassen.

    Das ist übertrieben. using in Headern ist (in 99% der Fälle) böse, aber in Source-Dateien macht das überhaupt nichts, weil die Auswirkungen in einem kontrollierbaren Bereich bleiben.



  • Carstig schrieb:

    statt assert würde ich eher eine exception werfen.
    Assert beendet doch das programm, oder? Während eine Exception dann vom Aufrufer behandelt werden könnte.

    Wie willst Du einen Programmierfehler behandeln?
    Nee, assert ist hier das Mittel der Wahl. Wir betreiben ja kein Java.



  • volkard schrieb:

    Nee, assert ist hier das Mittel der Wahl. Wir betreiben ja kein Java.

    Sehe ich aber auch anders.
    Es ist doch völlig okay in einer (vorhandenen) leeren Menge nach dem Maximum zu fragen.
    Ich krieg ja auch kein assert, wenn ich in einem leeren String suche.
    Ich dachte eher daran die Funktion in z.B.

    bool findMax(int& result) const {...}
    

    umzuändern.



  • Gut das const hatte ich ganz vergessen:

    class IntMenge {
    
    private:
    	std::vector<int> Menge;
    public:
    	void hinzufuegen(const int el);
    	void entfernen(const int el);
    	bool istMitglied(const int el) const;
    	size_t size() const;
    	void anzeigen() const;
    	void loeschen();
    	int getMax() const;
    	int getMin() const;
    	void sortieren();
    };
    

    Noch eine Frage. Was hat es denn für einen Vorteil meine eigene Methode size() anstatt Menge.size() zu nutzen?



  • ich sehe gerade...sollte vor den Rückgabetyp auch ein const?



  • volkard schrieb:

    Carstig schrieb:

    statt assert würde ich eher eine exception werfen.
    Assert beendet doch das programm, oder? Während eine Exception dann vom Aufrufer behandelt werden könnte.

    Wie willst Du einen Programmierfehler behandeln?
    Nee, assert ist hier das Mittel der Wahl. Wir betreiben ja kein Java.

    Ich dachte eher an den Anwendungs-Fall, wenn getMin/Max auf eine leere Menge aufgerufen wird. Dann würde ich statt 0 zurückzugeben eher eine Exception werfen.
    Denn 0 könnte ja, bei einer Menge mit nur einem Element mit dem Wert 0, durchaus ein korrekter Min/Max-Wert sein.
    ... bei einer leeren Menge entspricht 0 nicht wirklich dem Min/Max-Wert.



  • Mario.. schrieb:

    Noch eine Frage. Was hat es denn für einen Vorteil meine eigene Methode size() anstatt Menge.size() zu nutzen?

    Da 'Menge' richtigerweise private ist, kannst du Menge.size() ja ausserhalb der Klasse gar nicht nutzen.

    ch sehe gerade...sollte vor den Rückgabetyp auch ein const?

    Nein.



  • Das sind ja aber alles Klassenmethoden und da kann ich ja auf die Private Eigenschaften zugreifen. Und wieso sollte vor den Rückgabetyp kein const?



  • ... bei einer leeren Menge entspricht 0 nicht wirklich dem Min/Max-Wert.

    Sicher nicht, aber ein Mathematiker haut dir auch keine runter, wenn du nach dem
    Maximum einer Menge fragst, die kein Maximum hat.
    Das ist keine Ausnahme sondern was normales und legitimes.

    @Mario..:
    Ach verlesen, sry. In deinen eigenen Funktionen ist das so'ne Sache.
    Ggf. tauscht du z.B. vector durch ein Array (was zwar doof wäre, aber möglich).
    Dann müsstest du nicht deinen kompletten Code ändern, sondern nur die Size-Funktion.
    Aber wenn ich mir sicher bin, dass sich da nix ändert, dann rufe ich auch direkt size vom vector auf.



  • Mario.. schrieb:

    ich sehe gerade...sollte vor den Rückgabetyp auch ein const?

    All Deine Funktionen, die einen Rückgabewert liefern, geben diesen als "Value" zurück.
    Würden sie Referenzen auf interne, private Member zurückliefern, würde ein const Sinn machen.

    zb:

    const std::vector<int> & getPrivateVector() { return Menge; }
    

    (über die Sinnhaftigkeit obiger Funktion würde ich jetzt aber nicht diskutieren wollen :D)



  • Jockelx schrieb:

    ... bei einer leeren Menge entspricht 0 nicht wirklich dem Min/Max-Wert.

    Sicher nicht, aber ein Mathematiker haut dir auch keine runter, wenn du nach dem
    Maximum einer Menge fragst, die kein Maximum hat.
    Das ist keine Ausnahme sondern was normales und legitimes.

    Mathematiker sind ja auch Weicheier <duck und renn>
    Nein im Ernst: Ich glaube ein Mathematiker (Los gebt Euch zu erkennen) würde widersprechen, wenn ich sage, der kleinste Wert aller Elemente (getMin) einer leeren Menge ist 0.

    Wie man dann diesen Ausnahme/Normalfall in das Methodendesigen einbringt ist Geschmacksache.



  • Carstig schrieb:

    (Los gebt Euch zu erkennen)

    Hier!

    Carstig schrieb:

    würde widersprechen, wenn ich sage, der kleinste Wert aller Elemente (getMin) einer leeren Menge ist 0.

    Natürlich würde er das. Das haben wir ja lange geklärt.
    Es gibt aber unendlich viele Mengen ohne Minimum und wenn diese Klasse Mengen darstellt,
    soll sie mir bitte diesen Normalfall nicht mit einem assert oder einer Exception quittieren,
    wenn ich danach frage.


Log in to reply