[GELÖST] Problem mit istream

StringMath-Projekt

Ich hab was Größeres vor. Und weil ich Übung brauche und den Code von diesem Projket (Siehe Name und Signatur) geheim halten will, habe ich mir überlegt, vorab eine kleine Klasse zu machen und dort alle Grundtechniken anzuwenden, die ich brauche.

Jetzt habe ich ein Problem mit istream. Ich möchte für diese Klasse (Bruch) den Operator >> definieren damit man auch Daten z.B über die Konsole mit cin einlesen kann.

ich habe überhaupt keine Ahnung, wie ich das machen soll und frage euch deshalb um Hilfe.

Hier mal der Klassen Code:

#include <iostream>
#include <string>
#include <sstream>

using namespace std;

class Bruch
{
private:
	long intern_zaehler;
	long intern_nenner;

	void makeBruch(double);

public:
	Bruch();
	Bruch(short, short = 1);
	Bruch(int, int = 1);
	Bruch(long, long = 1);
	Bruch(long long, long long = 1);
	Bruch(float);
	Bruch(double);
	Bruch(const Bruch&);

	Bruch kuerzen();
	long GetZaehler();
	long GetNenner();
	void SetZaehler(long);
	void SetNenner(long);

	Bruch &operator=(double);
	Bruch &operator=(float);
	Bruch &operator=(short);
	Bruch &operator=(int);
	Bruch &operator=(long);
	Bruch &operator=(long long);

	operator double();
	operator float();
	operator short();
	operator int();
	operator long();
	operator long long();

	friend ostream& operator<<(ostream&, const Bruch&);
	friend istream& operator>>(istream&, Bruch&);
};

Und hier das Problem Kind (der ostream-Operator << poste ich mal mit.)

ostream& operator<<(ostream& Stream, const Bruch& obj)
{
	return Stream << obj.intern_nenner << "/" << obj.intern_zaehler;
}

istream& operator<<(istream& Stream, Bruch& obj)
{
	string str, tmp;
	int pos;
	long zahl;
	double zahl2;
	stringstream sstream;

	Stream >> str;

	pos = str.find("/");

	if(pos != -1)
	{
		tmp = str.substr(0, pos - 1);

		sstream << tmp;

		sstream >> zahl;

		obj.SetZaehler(zahl);

		tmp = str.substr(pos + 1);

		sstream << tmp;

		sstream >> zahl;

		obj.SetNenner(zahl);
	}
	else
	{
		sstream << str;

		sstream >> zahl2;

		obj = zahl2;
	}

	return NULL;
}

Bitte nicht schlagen, ich wirklich keine Ahnung von istream.

Danke schonmal für eure Hilfe.

P.S.: Ich hoffe mal, dass das nicht zu schwer ist.

CStoll

Die Pfeile in Zeile 6 zeigen in die verkehrte Richtung (der Eingabe-Operator ist >>)

StringMath-Projekt

Danke, aber das änder (fast) garnichts.

314159265358979

Also ich würde ja zuerst mit einem nicht ganz so geheimen Projekt anfangen, damit du wen fragen kannst bei Problemen.

CStoll

StringMath-Projekt schrieb:

Danke, aber das änder (fast) garnichts.

Mehr Hilfe kann dir meine Kristallkugel leider nicht anbieten. Für weitere Ratschläge solltest du etwas mehr Informationen liefern (z.B. welcher Fehler eigentlich auftritt).

StringMath-Projekt

Das hilft auch nichts, weil ich mir ziemlich sicher bin, dass ich das ganze von Grund auf falsch angehe.

Weil wenn ichs richtig verstanden habe, muss die Funktion einen Wert vom Typ istream zurückgeben.

@314159265358979 :

1. Deshalb läuft dieses Projekt. Hier übe ich.
2. Wenns dann dochmal Probleme gibt, werde ich den Code geschickt manipulieren, dass keiner ohne den Rest was damit anfangen kann, außer das Problem zu lösen.

Ich hoffe das hier findet einer, der sich mit sowas auskennt.

CStoll

Hast du denn den Hinweis umgesetzt aus meiner ersten Antwort? Der Eingabe-Operator heißt "istream& operator>>(istream& Stream, Bruch& obj)". Und ja, er sollte auch einen Stream zurückgeben für längere Operatorketten - normalerweise nimmt man dazu den, mit dem man die ganze Zeit gearbeitet hat:

istream& operator>>(istream& Stream, Bruch& obj)
{
  //... Einlesen und interpretieren der Daten
  return Stream;
}

(PS: Wenn du das nächste Mal ein Problem hast, schreib dazu welche Fehler der Compiler ausspuckt bzw. wie genau sich dein Problem äußert)

StringMath-Projekt

Danke hat geklappt.

1. Ja es muss den Stream zurückliefern (return Stream)
2. Ich hatte vergessen, das der Operator Bruch &operator=(double); noch nicht definiert war.

Danke für die Hilfe.

Die Funktion sieht jetzt so aus:

istream& operator>>(istream& Stream, Bruch& obj)
{
	string str, tmp;
	int pos;
	long zahl;
	double zahl2;
	stringstream sstream;

	Stream >> str;

	pos = str.find("/");

	if(pos != -1)
	{
		tmp = str.substr(0, pos - 1);

		sstream << tmp;

		sstream >> zahl;

		obj.SetZaehler(zahl);

		tmp = str.substr(pos + 1);

		sstream << tmp;

		sstream >> zahl;

		obj.SetNenner(zahl);
	}
	else
	{
		sstream << str;

		sstream >> zahl2;

		obj = zahl2;
	}

	return Stream;
}

Nexus

Zum letzten Code:

Deklariere die Variablen erst dort, wo du sie brauchst, und nicht am Anfang der Funktion.
Nimm std::string::npos statt der Magic Number -1 .
Weniger wichtig: Lass die unnötigen Leerzeilen weg und verwende einheitliche Namenskonventionen ( Stream vs. obj ).

Zu deiner Bruch-Klasse:

Ich würde implizite Konvertierungen vermeiden, besonders zu Integertypen. Dazu kannst du die Konstruktoren explicit machen und die Konvertierungsoperatoren durch mehrere benannte Funktionen (oder ein Funktionstemplate) ersetzen.
Benenne die Parameter bei deinen Funktionsdeklarationen. Auch wenn es nicht nötig ist, sollte die öffentliche Schnittstelle so aussagekräftig wie möglich sein.
Den Zähler und Nenner eines Bruchs einzeln zu setzen halte ich nicht für sinnvoll. Biete wenn überhaupt eher eine Methode an, die beides auf einmal setzt. Aber mit dem Zuweisungsoperator ist das eigentlich auch unnötig.
Die Streamoperatoren müssen nicht befreundet sein, da sie über das öffentliche Interface genügend Zugriff auf die Klasse haben.
Kopierkonstruktor musst du nicht deklarieren. Falls du es doch tust (an anderen Orten, wo es sinnvoll ist), halte dich an die Regel der Grossen Drei.
Deklariere Memberfunktionen, welche das Objekt nicht ändern, als const .

StringMath-Projekt

Nexus schrieb:

Deklariere die Variablen erst dort, wo du sie brauchst, und nicht am Anfang der Funktion.

Ich deklariere sie IMMMER am Anfang, weil ich dann immer weiß, welche Variablen die Funktion hat.

Nexus schrieb:

Nimm std::string::npos statt der Magic Number -1 .

Warum? Macht das so einen großen Unterschied?

Nexus schrieb:

Weniger wichtig: Lass die unnötigen Leerzeilen weg und verwende einheitliche Namenskonventionen ( Stream vs. obj ).

Die Leerzeichen dienen zur bessern Übersicht. Da achte ich sehr drauf. (Wartung, Codeverständnis, ...) Und das mit den Namen, da hast du recht, aber in dem moment ist mir kein besserer Name eingefallen.

Nexus schrieb:

Ich würde implizite Konvertierungen vermeiden, besonders zu Integertypen. Dazu kannst du die Konstruktoren explicit machen und die Konvertierungsoperatoren durch mehrere benannte Funktionen (oder ein Funktionstemplate) ersetzen.

Das kenne ich leider nicht. Und weiß nicht was du meinst.

Nexus schrieb:

Benenne die Parameter bei deinen Funktionsdeklarationen. Auch wenn es nicht nötig ist, sollte die öffentliche Schnittstelle so aussagekräftig wie möglich sein.

Kann ich machen

Nexus schrieb:

Den Zähler und Nenner eines Bruchs einzeln zu setzen halte ich nicht für sinnvoll. Biete wenn überhaupt eher eine Methode an, die beides auf einmal setzt. Aber mit dem Zuweisungsoperator ist das eigentlich auch unnötig.

Das "Projekt ist nur zum Üben da. Die Idee hab von dieser Seite (http://www.willemer.de/informatik/cpp/cppovrld.htm). Da hab ich das mit den Operatoren nachgeschaut.

Nexus schrieb:

Die Streamoperatoren müssen nicht befreundet sein, da sie über das öffentliche Interface genügend Zugriff auf die Klasse haben.

Ich weiß, aber dann kann ich auf alle Memberfunktionen und -variablen zugreifn, was unter Umständen sehr praktisch ist.

Nexus schrieb:

Kopierkonstruktor musst du nicht deklarieren. Falls du es doch tust (an anderen Orten, wo es sinnvoll ist), halte dich an die Regel der Grossen Drei.

Davon hab ich leider auch keine Ahnung. Ein Beispiel oder eine Erleuterung wäre praktisch.

Nexus schrieb:

Deklariere Memberfunktionen, welche das Objekt nicht ändern, als const .

Kann man machen, muss man aber nicht...

ioch danke dir mal für dein sehr konstruktives Kommentar.

CStoll

StringMath-Projekt schrieb:

Nexus schrieb:

Deklariere die Variablen erst dort, wo du sie brauchst, und nicht am Anfang der Funktion.

Ich deklariere sie IMMMER am Anfang, weil ich dann immer weiß, welche Variablen die Funktion hat.

Das klingt nach einem alten C-Veteranen. Ich muß (in SAL) auch alle Variablen einer Funktion zentral definieren - und du glaubst gar nicht, wie schnell man auf diese Weise den Überblick verliert.

Nexus schrieb:

Nimm std::string::npos statt der Magic Number -1 .

Warum? Macht das so einen großen Unterschied?

Es sagt klar aus, was du haben willst. Außerdem ist npos (genau wie der Rückgabewert von find()) ein vorzeichenloser Wert.

Nexus schrieb:

Weniger wichtig: Lass die unnötigen Leerzeilen weg und verwende einheitliche Namenskonventionen ( Stream vs. obj ).

Die Leerzeichen dienen zur bessern Übersicht. Da achte ich sehr drauf. (Wartung, Codeverständnis, ...) Und das mit den Namen, da hast du recht, aber in dem moment ist mir kein besserer Name eingefallen.

Leerzeichen sind kein Problem, nach jedem Befehl eine LeerZEILE einzubauen zieht deinen Code nur unnötig in die Länge. Und bei den Namen reicht es ja schon, wenn du die Groß- und Kleinschreibung einheitlich durchsetzt

Nexus schrieb:

Die Streamoperatoren müssen nicht befreundet sein, da sie über das öffentliche Interface genügend Zugriff auf die Klasse haben.

Ich weiß, aber dann kann ich auf alle Memberfunktionen und -variablen zugreifn, was unter Umständen sehr praktisch ist.

Auf die internen Daten der Klasse sollte nur derjenige Zugriff bekommen, der es unbedingt benötigt. Auf diese Weise minimierst du das Risiko, versehentlich die Klassen-Invarianten kaputtzumachen.

Nexus schrieb:

Kopierkonstruktor musst du nicht deklarieren. Falls du es doch tust (an anderen Orten, wo es sinnvoll ist), halte dich an die Regel der Grossen Drei.

Davon hab ich leider auch keine Ahnung. Ein Beispiel oder eine Erleuterung wäre praktisch.

Der Compiler legt den Kopier-Konstruktor, Zuweisung und Destruktor selber an, wenn sie nicht existieren - bei deiner Bruch-Klasse machen auch das richtige, um sie verwenden zu können. Bei Klassen, wo du einen von den dreien manuell definieren mußt, weil die compilergenerierte Version nicht ausreicht (z.B. wegen eigener Speicherverwaltung), benötigst du normalerweise auch die anderen beiden (z.B. wenn der Destruktor etwas per delete freigeben muß, müssen die Kopier-Methoden so geschrieben werden, daß du nichts doppelt freigibst).

Nexus schrieb:

Deklariere Memberfunktionen, welche das Objekt nicht ändern, als const .

Kann man machen, muss man aber nicht...

Doch, man sollte es schon. Du hast z.B. in deinem Ausgabe-Operator ein konstantes Objekt übergeben - und für dieses kannst du nur die const-Methoden verwenden.

Nexus

StringMath-Projekt schrieb:

Ich deklariere sie IMMMER am Anfang, weil ich dann immer weiß, welche Variablen die Funktion hat.

Wozu sollte man das wissen?

Der Vorteil von späten Deklarationen ist einerseits, dass der Scope lokal ist und dadurch weniger Möglichkeiten bestehen, auf die Variable ausserhalb ihres Anwendungsbereiches zuzugreifen. Andererseits hat man gleich sinnvolle Initialwerte. Gewisse Klassen sind nämlich nicht defaultkonstruierbar, und dann müsstest du am Anfang irgendein Dummy-Objekt erstellen, wenn du noch zu wenig Informationen für ein richtiges hast.

StringMath-Projekt schrieb:

Warum? Macht das so einen großen Unterschied?

Ja, man sieht gleich, was gemeint ist. Wenn die Konstante schon extra dafür existiert, benutze sie doch.

StringMath-Projekt schrieb:

Die Leerzeichen dienen zur bessern Übersicht. Da achte ich sehr drauf. (Wartung, Codeverständnis, ...)

Tue ich auch, aber normalerweise gruppiere ich einige Zeilen und mache nicht zwischen jeder Zeile einen Abstand. Finde ich persönlich übersichtlicher, weil man damit automatisch eine Gliederung hat.

StringMath-Projekt schrieb:

Und das mit den Namen, da hast du recht, aber in dem moment ist mir kein besserer Name eingefallen.

"Stream" als Name ist schon okay, aber ich würde ihn wie andere Variablen klein schreiben.

StringMath-Projekt schrieb:

Das kenne ich leider nicht. Und weiß nicht was du meinst.

Konvertierungsoperatoren und das explicit -Schlüsselwort sollten in deinem C++-Buch erklärt sein. Mit benannten Funktionen meine ich sowas wie float ToFloat() const statt operator float() const .

StringMath-Projekt schrieb:

Das "Projekt ist nur zum Üben da.

Das hört man leider sehr häufig. Doch warum nicht gleich richtig üben? Falsche Dinge anzugewöhnen bringt dir nichts.

StringMath-Projekt schrieb:

Die Idee hab von dieser Seite (http://www.willemer.de/informatik/cpp/cppovrld.htm). Da hab ich das mit den Operatoren nachgeschaut.

Ich hab kurz ein paar Seiten angeschaut und den Eindruck bekommen, als würde einiges nur oberflächlich erklärt. Wundert mich bei Online-Tutorials übrigens nicht. Ich würde mir unbedingt ein gutes C++-Buch zu tun. Z.B. C++-Primer, Accelerated C++ oder Thinking in C++ (auch gratis als E-Book).

StringMath-Projekt schrieb:

Ich weiß, aber dann kann ich auf alle Memberfunktionen und -variablen zugreifn, was unter Umständen sehr praktisch ist.

Dafür brichst du die Kapselung, das sind die geringen Vorteile nicht wert. Wenn du plötzlich die Implementierung änderst (z.B. die Membervariablen anders nennst), musst du nun auch die Streamoperatoren anpassen.

Halte Abhängigkeiten immer so gering wie möglich. Das public -Interface der Klasse sollte klein sein, und wenn möglich soll der gesamte Zugriff von aussen darüber erfolgen. Dadurch kannst du diesen auch besser kontrollieren und hast eine Abstraktion von Implementierungsdetails.

StringMath-Projekt schrieb:

Davon hab ich leider auch keine Ahnung. Ein Beispiel oder eine Erleuterung wäre praktisch.

Du hast den Kopierkonstruktor ja hingeschrieben. Kopiere keinen Code, den du nicht verstehst. Kopierkonstruktoren werden übrigens auch in guten C++-Büchern erklärt.

StringMath-Projekt schrieb:

Kann man machen, muss man aber nicht...

Natürlich muss man nicht, man muss auch keine Klassen verwenden. const -Correctness hat wesentliche Vorteile. Welche, steht wiederum im C++-Buch.

Edit: Verdammt, CStolls Post nicht gesehen
Naja, nun hast du immerhin eine Bestätigung. Vielleicht wurde auch was noch nicht gesagt...

StringMath-Projekt

So ich hab mir das jetzt mal durchgelesen und eure Positionen verstanden.

Nur noch ein paar Dinge:

Die Konstante std::string::npos kannte ich nicht. Ich werde sie jetzt auch verwenden.
Die implizite Konvertierung ist gewollt.
Ich hatte das mit "den Großen Drei" nicht verstanden. (Weil ich das so nicht kannte) Und den Kopierkonstruktur kann ich dann wirklich rausnehmen, nur die Leere Zuweisung(Bruch ohne Argumentenliste) bleibt, weil die die beiden long-Werte nicht 0 sein dürfen. Der zaehler MUSS nämlich 1 sein.

Sonst habe ich alles verstanden.

hustbaer

Wie tu ich dann Null in deine Klasse rein, wenn der Zähler 1 sein muss?
Ich denke mal du hast das verwechselt: der Nenner darf nicht Null sein.

0/1 = 0
1/0 = ....

StringMath-Projekt

Stimmt! Hab ich in dem Meont ganz verwechselt.

CStoll

StringMath-Projekt schrieb:

Ich hatte das mit "den Großen Drei" nicht verstanden. (Weil ich das so nicht kannte) Und den Kopierkonstruktur kann ich dann wirklich rausnehmen, nur die Leere Zuweisung(Bruch ohne Argumentenliste) bleibt, weil die die beiden long-Werte nicht 0 sein dürfen. Der zaehler MUSS nämlich 1 sein.

Der Default-Konstruktor ist auch kein Problem. Aber ansonsten kannst du auch die impliziten Umwandlungen nutzen, die der Compiler bietet - und alle Ganzzahl-Konstruktoren zu einem zusammenfassen: Bruch(long long z=0,long long n=1); (das erschlägt auch den Default-Konstruktor). Was die float/double-Konstruktoren machen sollen, bin ich mir auch nicht sicher.
Auf der anderen Seite dürfte die implizite Umwandlung nach int ein Problem darstellen, wenn du mit gemischten Operationen zu tun hast (wird der Ausdruck "i*b" jetzt mit int-Arithmetik oder mit Bruch-Operatoren berechnet?).

StringMath-Projekt

Ich arbeit gerade daran die Rechenoperatoren zu interpretieren.
Und die float/double-Konstruktoren werden dann zu Brüchen generiert.

Also wenn man folgend Code hat:

...

int main()
{
    Bruch br(2.58);

    cout << br << endl;

    return 0;
}

Dann kommt dabei das raus.

17/8

Ich finde diese automatische Umwandlung sehr praktisch, deswegen habe ich sie interpretiert.

CStoll

StringMath-Projekt schrieb:

Ich arbeit gerade daran die Rechenoperatoren zu interpretieren.

Da sind dann die operator int() etc. eher ein Problem - und erzeugen Mehrdeutigkeiten:

bruch b(1,2);
int i = 5;
cout << i + b << endl;

Hier kann sich der Compiler nicht entscheiden, ob er i in den Bruch 5/1 umwandelt und deinen Operator nutzt oder b in einen int umwandelt und per eingebautem Operator die Summe berechnet.

Dann kommt dabei das raus.

17/8

Dann versuch das mal mit der Eingabe 0.1

StringMath-Projekt

Dann kommt 1/10 raus...

CStoll

StringMath-Projekt schrieb:

Dann kommt 1/10 raus...

Geraten oder ausprobiert? (wenn letzteres, würde mich mal interessieren, wie der double-Konstruktor aussieht)