[gelöst] 'cout<<' von 'vector <char*>' ist blanc

kaizimir

Moin,

warum, zum Donnerdrummel , kriege ich bei dem Code:

KvDataAscii::KvDataAscii(string& StrFileName)
{
    ifstream StreamData(StrFileName.data());
	vector <char*> M_vDataValue;

    while ( StreamData.getline(m_chDataValue, 256, 0x09) )
    {
        M_vDataValue.push_back(m_chDataValue);
    }
    int myint = M_vDataValue.size();

	cout<<myint<<endl;
	cout<<M_vDataValue[2]<<endl;
	cout<<"value"<<endl;
}

diese Ausgabe:

./DataAnalyse
582214

value
***Normal beendet***

Eigentlich interessiert mich nur der Inhalt des Vektors, aber die Zeile ist blanc. Wahrscheinlich nur ein Anfängerfehler, aber ich komm nicht drauf. Kann mir bitte jemand auf die Sprünge helfen.

Danke, kai

Simon2

... weil beim letzten Lesen des Streams (da, wo das "end-of-file" oder "file-not-found" oder ... gefunden wird) die getline()-Implementierung netterweise genau das zurückgibt.

Du hast doch schon string dabei - dann nutze ihn doch auch (wie übrigens auch die cpp-Tags). Wenn Du nur Zeiger auf ein (immer dasselbe!) char-Array ablegst, ist das garantiert nicht das, was willst.

(Ich hoffe inständig, dass m_chDataValue ein char-Array ist!! Wenn es nur ein char* ist, ist der Code trotzdem kaputt)

Mal eine erste Version, die nur den eine Fehler korrigiert:

KvDataAscii::KvDataAscii(string& StrFileName)
{
   ifstream StreamData(StrFileName.data());
   vector <string> M_vDataValue; // nicht char*

   while (StreamData.getline(m_chDataValue, 256, 0x09) ) {
      M_vDataValue.push_back(m_chDataValue); // aus dem char[] wird direkt ein string konstruiert und an M_vDataValue angehängt
   }
...

Besser noch (spontan):
- Für den Fall, dass man einen string hat und einen char const* braucht, ist string::c_str() gedacht nicht string::data(); das könnte in (zugegebenermaßen sehr seltenen) Fällen zu Problemen führen.
- Gleich einlesen in string (kann nicht zu Buffer-overflows führen); gibt ein eigenes getline() dafür
- Wenn m_chDataValue zu nichts Anderes gut ist, als temporärer Buffer zu sein, sollte es keine Membervariable sein.
- Da Du StrFileName seeeeehr wahrscheinlich nicht mändern willst, sollte die Referenz darauf const sein (auch in der der Funktionsdeklaration)
- den Umweg über myint brauchst Du nicht
- Du solltest prüfen, ob überhaupt 3 Elemente im Vektor sind, bevor Du zugreifst - oder vector::at() verwenden, weil der die Prüfung übernimmt (und anderenfalls eine exception wirft.
- ich persönlich bevorzuge Leerzeichen zwischen einzelnen Ausgabetoken und Zeilenumbrüche da, wo welche sein sollen (und endl nur da, wo auch wirklich geflusht werden soll).

nächster Anlauf:

KvDataAscii::KvDataAscii(string const& StrFileName)
{
    ifstream StreamData(StrFileName.c_str());
    vector <string> M_vDataValue;

    string tmp;
    while (getline(StreamData, tmp, 0x09) M_vDataValue.push_back(tmp);
    cout<< M_vDataValue.size() << ' ' << M_vDataValue.at(2) << "\nvalue"<<endl;
}

Noch eine Rückfrage: Ist es wirklich notwendig/sinnvoll, 0x09 als Delimiter zu verwenden?
Kann sein, kann auch nicht sein ....

Gruß,

Simon2.

kaizimir

Hi Simon2,

vielen Dank für den ausführlichen Post. Deine Verbesserungsvorschläge nehme ich gerne in den Code auf.

Simon2 schrieb:

(Ich hoffe inständig, dass m_chDataValue ein char-Array ist!! Wenn es nur ein char* ist, ist der Code trotzdem kaputt)

Jou, das ist ein Array.

Eigentlich brauch ich auch keinen string, sondern floats. Bin halt gerade dabei mir das zusammen zu basteln.

Ok, eigentlich war die Frage eine andere, aber durch deine Rückfragen halte ich es für sinnvoll, die Situation einmal zu schildern:
In dem ascii-file stehen Kommawerte in 4 Spalten. Die Spalten sind durch Tabs getrennt (daher 0x09). Die Werte sollen eingelesen werden (dürften etwa 4x500.000 sein) und am besten in Variablen/einem Struct für jede Spalte als Float gespeichert werden.

Meine Idee war:
Den Stream in den Vektor einlesen, mit einer Schleife die Werte aus dem Vektor den richtigen Variablen zu weisen und gleichzeitig konvertieren. Diese Vorgehensweise ist wahrscheinlich ziemlich umständlich, aber erschließbar, wenn man noch nicht die ganzen Möglichkeiten von streams kennt.

Noch eine Frage hier zu:

Simon2 schrieb:

Wenn m_chDataValue zu nichts Anderes gut ist, als temporärer Buffer zu sein, sollte es keine Membervariable sein.

Was ist das besondere an Membervariablen bzw. welchen unterschied macht es eine Variable schon im header zu deklarieren?

greetz,
kai

Simon2

Hi kaizimir,

OK, dann hätte ich noch ein wenig für Dich.
Erstmal die Frage: liegen die floats "binär" vor?
Oder einfach ausgedrückt: Kannst Du das File mit einem normalen Editor "lesen"?
Steht da:

1.23   45.6    2312.0   3.1

?
Oder sind da "nichtdruckbare Binärwerte drin?

Im ersten Fall hast Du's deutlich leichter (merke: Binärdarstellungen sind in C++ nicht normiert - sie können sich von z.B. Compiler zu Compiler unterscheiden).
Auf jeden Fall kannst Du floats auch direkt einlesen (und brauchst Dich nicht einmal um das konkrete "whitespace"-Zeichen zu kümmern):

...
    float tmp1, tmp2, tmp3, tmp4;
    while (StreamData >> tmp1 >> tmp2 >> tmp3 >> tmp4) {
      // liest solange 4 floats ein, wie es geht
...

Schritt 1 könnte also sein:

struct Dimensions {
   float laenge, breite, hoehe, tiefe;
};

KvDataAscii::KvDataAscii(string const& StrFileName)
{
    ifstream StreamData(StrFileName.c_str());
    vector <Dimensions> M_vDataValue;

    Dimensions tmp;
    while (StreamData >> tmp.laenge >> tmp.breite >> tmp.hoehe >> tmp.tiefe) M_vDataValue.push_back(tmp);
...

Richtig nett "C++ig" wird's dann aber mit eigenem operator>>() und operator<<():

istream& operator>>(istream& in, Dimensions& d) {
   return in >> d.laenge >> d.breite >> d.hoehe >> d.tiefe;
}

ostream& operator<<(ostream& out, Dimensions const& d) {
   return out << '<' << d.laenge << ", " << d.breite << ", " << d.hoehe << ", " <<  d.tiefe << '>';
}

KvDataAscii::KvDataAscii(string const& StrFileName)
{
    ifstream StreamData(StrFileName.c_str());
    vector<Dimensions> M_vDataValue;

   // Einlesen:
    Dimensions tmp;
    while (StreamData >> tmp) M_vDataValue.push_back(tmp);

   // Ausgeben:
   for(vector<Dimensions>::const_iterator it=M_vDataValue.begin(); it != M_vDataValue.end(); ++it) 
      cout << *it << '\n';
...

... oder dann mit Iteratoren:

KvDataAscii::KvDataAscii(string const& StrFileName)
{
   ifstream StreamData(StrFileName.c_str());

   vector <Dimensions> M_vDataValue;
   // Einlesen:
   copy(istream_iterator<Dimensions>(StreamData),  istream_iterator<Dimensions>(), back_inserter(M_vDataValue));

   // Ausgeben:
   copy(M_vDataValue.begin(), M_vDataValue.end(), ostream_iterator<Dimensions>(cout, "\n"));

Aber das ist schon eher abgefahren und wird nicht mehr von Jedem als "übersichtlich" gewertet

kaizimir schrieb:

...Was ist das besondere an Membervariablen bzw. welchen unterschied macht es eine Variable schon im header zu deklarieren? ...

Du legst jeden fest, der diese Klasse irgendwann einmal benutzen will.
Wenn Du später mal etwas in der Klassendeklaration änderst, funktioniert ersteinmal kein Programm mehr, das mit der alten Deklaration gebaut wurde. Im Idealfall muss nur jedes Programm neu compiliert, gelinkt und ausgeliefert werden (kann auch schon mühsam sein). Im schlechten Fall hat aber irgendjemand Anders diese Membervariable bereits in seinem Code verwendet - mit dem musst Du Dich nun zusammensetzen und auskaspern, ob/wie er sein Programm entsprechend ändert ...

Außerdem fragt sich jemand, der Deine Klasse ansieht, wie er sie benutzen muss - und wenn dann da Member auftauchen, mit denen er gar nichts zu tun hat, ist das extrem verwirrend.
Insgesamt gilt: Am Besten Dinge möglichst "lokal" deklarieren.
... und im konkreten Fall: In Klassen nur das Allernötigst - und keine Implementierungsdetails.

Gruß

Simon2.

kaizimir

Hi Simon2,

vielen Dank für deine Hilfe. Sieht wirklich geschmeidig aus und funktioniert einwandfrei. Der erste Vorschlag gefällt mir besonders gut. Den versteh ich auch komplett ohne größere Gehirnverrenkungen. Sollte der Konstruktor doch noch verfeinert werden müssen, komme ich sicherlich noch mal auf die anderen Codestücke zurück.

Das einzige was für mich noch heraus zu finden blieb, war, wie man nun auf die Elemente zurückgreift, zB:

cout << m_vDataValue.at(10).time<<endl;

Auch die Erklärung zu den Membervariablen war sehr hilfreich. Da kann man sich ja ziemlich einfach dran halten.

Ich schließ mich jedenfalls deiner Signatur an und bin heute der, der davon profitiert.

greetz, kai

Simon2

Hi,

Danke für die nette Rückmeldung.

kaizimir schrieb:

...
Das einzige was für mich noch heraus zu finden blieb, war, ...

Naja, Dir soll ja auch nicht langweilig werden.

Alles Gute,

Simon2.

kaizimir

Hey,
cool, dass du noch da bist. Ich würde nämlich gerne noch zwei, drei Fragen stellen. Ich poste mal den ganzen Code. Ich hoffe, dass ist nicht zu viel.

Erstmal die Implementierung:

#include "KvDataAscii.h"

struct Dimensions
{
	vector <float> time, trigger, stimulus, recording;
};

KvDataAscii::KvDataAscii()
		: KvData()
{
}

KvDataAscii::KvDataAscii ( const string& strFileName )
{

	if ( strFileName.empty() )
	{
		cerr<<"No file name present"<<endl;
		return;
	}

	ifstream streamData ( strFileName.c_str() );
	float time, trigger, stimulus, recording;

	while ( streamData >> time >> trigger >> stimulus >> recording )
	{
		vValues.time.push_back ( time );
		vValues.trigger.push_back ( trigger );
		vValues.stimulus.push_back ( stimulus );
		vValues.recording.push_back ( recording );
	}
}

const struct Values* KvDataAscii::getData() const
{
	return &vValues;
}

KvDataAscii::~KvDataAscii()
{
}

Der Header:

#ifndef KVDATAASCII_H
#define KVDATAASCII_H

#include <iostream>
#include <string>
#include <fstream>

#include <KvData.h>

struct Values
{
	vector <float> time, trigger, stimulus, recording;
};

class KvDataAscii : public virtual KvData
{

// Methodes
public:
	KvDataAscii();
	KvDataAscii ( const string& );

	const struct Values* getData() const;

	~KvDataAscii();

// Member variables
private:
	Values vValues;
};

#endif

Das virtuelle Interface:

#ifndef KVDATA_H
#define KVDATA_H

#include <vector>

using namespace std;

struct Values;

class KvData
{
// Methodes
public:
    virtual const struct Values* getData() const = 0;

    virtual ~KvData() {};
};

#endif

Die aufrufende Methode, die :

void KvMainWindow::open()
{
	QString qstrFileName = QFileDialog::getOpenFileName ( this );
	if ( !qstrFileName.isEmpty() )
	{
		string strFileName ( qstrFileName.toStdString() );

		KvDataAscii asciiData ( strFileName );

		const Values* vValues;
		vValues = asciiData.getData();

	}
}

So, also erstmal habe ich das struct so umgebaut, dass es nun 4 vectoren hält. So bin ich das aus Matlab gewohnt. Entstehen dadurch Nachteile auf dem Stack? Wäre es überhaupt sinnvoll das struct auf den Heap zu legen? Im späteren Programmablauf wird sich fast alles um diese Daten drehen.

Ich hab das virtuelle Interface gebastelt, da später noch andere Dateiformate geladen werden sollen. Ich denke, ich werde es später einfach zu einer Basisklasse umbauen. Da ich zur Compilezeit noch nicht weiß, welchen Dateityp die open()-Methode öffnen soll, müsste das noch geändert werden. Richtig gedacht?

Sind da grobe Schnitzer in dem Code? Vor allem bin ich mir bei den Deklarationen des 'struct' nicht sicher, ob das so richtig ist.

Ich hoffe, ich bin nicht zu aufdringlich und freue mich jedenfalls über Kritik und Anregungen.

greetz, kai

Simon2

Hi,

kaizimir schrieb:

...Entstehen dadurch Nachteile auf dem Stack? Wäre es überhaupt sinnvoll das struct auf den Heap zu legen? Im späteren Programmablauf wird sich fast alles um diese Daten drehen....

Da würde ich mir keine Sorgen drum machen.
Prinzipiell ist das implementationsabhängig - aber ich nenne mal den wahrscheinlichsten Fall:
Du kannst mit großer Wahrscheinlichkeit davon ausgehen, dass vector die "Daten" auf dem Heap ablegt und nicht auf dem Stack.
Die "Verwaltungsinformationen" (Zahl der Elemente, Zahl der freien Speicherplätze, Adresse der Daten, ...) wird zwar auf dem Stack abgelegt, aber das wird nicht soooo die Masse sein.

Ansonsten gefällt mir das schon richtig gut - habe es nur überflogen aber doch einen guten Einblick gewonnen.
Allerdings
- sehe ich nicht, warum Du "virtual" erben möchtest - das ist eigentlich eher etwas für spezielle Fälle bei geplanter Mehrfachvererbung und das kann ich hier nicht erkennen (und hat versteckte Fußangeln)...
- Bei der Zeigerrückgabe bei "getData()" habe ich kurz gezuckt ("Warum keine Referenz?") - das könnte evtl. auch mit Referenz gelöst werden - ist aber nicht unbedingt besser.

kaizimir schrieb:

...Da ich zur Compilezeit noch nicht weiß, welchen Dateityp die open()-Methode öffnen soll, müsste das noch geändert werden. Richtig gedacht? ...

Ja - das kannst Du dann in eine "Factory-Klasse" auslagern, die sich damit herumschlägt, den richtigen Typen zu erzeugen.

kaizimir schrieb:

...Vor allem bin ich mir bei den Deklarationen des 'struct' nicht sicher, ob das so richtig ist....

Ja, da ist Dir alte C-Syntax in die Quere gekommen... genau wie bei "class" stellt ein "struct" bereits den neuen Typen dar und braucht kein "struct"-keyword mehr bei der Benutzung. Es reicht also:

const Values* KvDataAscii::getData

(und so weiter)

Also: Eher Kleinigkeiten und schon richtig ordentlich.

Gruß,

Simon2.

[gelöst] 'cout&lt;&lt;' von 'vector &lt;char*&gt;' ist blanc

[gelöst] 'cout<<' von 'vector <char*>' ist blanc