Probleme beim Einlesen einer Datei



  • Hi!
    Ich schreibe an einem Programm, doch leider kann ich es nicht testen, da es immer mit dem Kommentar Speicherzugriffsfehler (Speicherabzug geschrieben) abstürzt.

    Der Code sieht folgendermaßen aus:

    using namespace std;
    #include <iostream>
    #include <fstream>
    #include <string>
    
    void access_value(int* value, const char* Filename)
    {
    	ifstream myfile;							// opens a filestream
    	string Line;
    	cout<<Filename<<endl;
    	cout<<"1"<<endl;
    	myfile.open(Filename, ios_base::in);		// opens a file
    	if (!myfile)
    	{
    		cout<<"Error 04 File not found."<<endl;	// returns an Error
    	}
    	cout<<"test"<<endl;
    	for (int i = 0; i < 95; i++)				// for every line in the file
    	{
    		myfile >> value[i];						// copies the line in the array
    		cout<<value[i]<<endl;
    	}
    	myfile.close();								// closes the file
    	cout<<value[36]<<endl;
    }
    

    In der Ausgabe in der Konsole (Ubuntu 16.04.3 LTS) wird die "1" noch korrekt ausgegeben, "test" hingegen schon nicht mehr. Ich vermute den Fehler also in Zeile 12.
    Was sonst noch wichtig sein könnte:
    - Die Datei welche eingelesen werden soll besteht aus den Zahlen 0 bis 94 jeweils eine Zahl in einer Zeile.
    - das Array value hat die 95 Einträge
    - mit cat [Filename] wird mir der richtige Inhalt der Datei in der Konsole angezeigt
    - ganz ähnlich sieht das Problem in der Ausgabe der Daten aus (siehe unterer Code). Dort wird "PF1" noch ausgegeben "PF2" nicht mehr. Es wird eine leere Datei mit dem richtigen Namen erzeugt.
    - Beide Funktionen habe ich in nahezu identischer Form in einem einwandfrei funktierenden Programm bereits verwendet.

    Code:

    using namespace std;
    #include <iostream>
    #include <fstream>
    #include <string>
    
    void printFile(int countLine, char* Filename, char** text)
    {
    	string* output;
    	output = new string[countLine];	
    	for(int i=0; i<countLine; i++)
    	{
    		output[i] = text[i];
    	}
    	string strFilename(Filename);									// converts the filename from char to string
    	string new_filename;											// defines a new var of typ string
    	new_filename = strFilename.substr(0, (strFilename.length() -4)) + "_neu.txt";
    	const char* char_new_filename = new_filename.c_str();			// converts new_filename from string to char*
    	ofstream myfile;												// creates an output stream
    	cout<<"PF1"<<endl;
    	myfile.open(char_new_filename, ios::app);						// opens the File char_new_filename
    	cout<<"PF2"<<endl;
    	for(int i = 0; i < countLine; i++)								// for every line
    	{
    		myfile << output[i]<<endl;									// writes the line into the file
    	}
    	myfile.close();													// closes the file
    }
    

    Die Fragen sind nun warum bekomme ich einen Speicherzugriffsfehler? und noch wichtiger: was kann ich dagegen tun?



  • Wenn zwischen 2 couts offensichtlich korrekter Code steht, das 2. cout aber nicht mehr ausgeführt wird, ist der Fehler anderswo.

    Versuch doch mal, dein ganzes Programm mit -fsanitize=address zu kompilieren. Vielleicht kommt da schon schnell die Stelle raus. Ansonsten valgrind nehmen.

    Aber - viel wichtiger:
    das Programm meinst du doch so nicht ernst?! Was soll das ganze herumgepointere? Warum benutzt du nicht std::vector<int> und std::string statt dem int* und char* (wobei du bei den char*(*) nicht einmal ein const hast!) Und was soll das output string-Array? Brrr....

    Als Kommentar zu einer Funktion ist es außerdem hilfreich, nicht eine einzelne Zeile wie "// opens a filestream" zu kommentieren, sondern beschreibe lieber ruhig ausführlicher, was genau die jeweilige Funktion (nicht einzelne Zeilen einer Funktion) denn machen soll!


  • Mod

    • Keine Arrays benutzen, nimm std::vector oder std::array.
    • Keine Zeiger benutzen, nimm Referenzen.
    • Niemals irgendwie new in deinem Programm benutzen, nimm std::vector.

    Wenn du diese Tipps beherzigst, wird sich dein Problem von alleine lösen; du wirst nie wieder Probleme dieser Art haben; und du weist nun etwas über C++ (was genau eine Sache mehr ist als dein Lehrer über C++ weiß).

    Bezüglich allgemeiner Programmierung solltest du beachten: Eine Funktion macht genau eine Sache und die macht sie richtig! Vergleiche dazu, was in deinem Programm nicht stimmt: Die Funktion access_value tut folgende Dinge: Dateinamen ausgeben, '1' ausgeben, Datei öffnen, Fehlerbehandlung für Öffnung durchführen (aber falsch), Lesen von (einer merkwürdig spezifischen Anzahl von) Werten aus einer Datei (aber falsch), Werte ausgeben, Datei schließen, einen (merkwürdig spezifischen) Wert ausgeben. Die Funktion printFile ist genau so schlimm. Wenn du dies beherzigst, wirst du nie wieder Probleme mit der Struktur deiner Programme haben. Außerdem weißt du dann ein bisschen etwas über Programmierung, was genau eine Sache mehr ist, als dein Lehrer über Programmierung weiß.

    Vielleicht solltest du dir auch einen anderen Lehrer suchen. Oder falls das nicht in Frage kommt, ignorier ihn so gut es geht und such dir stattdessen ein gutes Buch. Das ist kein Scherz oder Übertreibung. Dein Lehrer hat wirklich überhaupt keine Ahnung und wird dir nur Müll beibringen, der nicht funktionieren wird und den du mühsam wieder verlernen musst, sofern du nicht lange vorher die Programmierung frustriert aufgibst. Das ist leider häufig so, weil niemand die Kompetenz eines Lehrers in solchen Dingen prüft.



  • Danke für eure schnellen Antworten!

    wob schrieb:

    Versuch doch mal, dein ganzes Programm mit -fsanitize=address zu kompilieren. Vielleicht kommt da schon schnell die Stelle raus. Ansonsten valgrind nehmen.

    Ok könntest du mir genauer beschreiben, wie das mit -fsanitize=address geht? Ist das ein Parameter?
    Momentan kompillere ich mit g++ *.cpp -o test
    und führe die mit ./test [Parameter1] [Parameter2] aus.
    Mit valgrind kenne ich mich noch gar nicht aus. Ich muss mich dazu mal belesen.

    wob schrieb:

    Aber - viel wichtiger:
    das Programm meinst du doch so nicht ernst?! Was soll das ganze herumgepointere? Warum benutzt du nicht std::vector<int> und std::string statt dem int* und char* (wobei du bei den char*(*) nicht einmal ein const hast!) Und was soll das output string-Array? Brrr....

    Das output Array war so eine Art Test. Es sollte eigentlich auch gehen, wenn man die Zeilen 8 - 13 entfernt und nachfolgend output durch text ersetzt.
    Warum ich int* und char* statt vector benutze? Weil ich es so gelernt habe und andere Möglichkeiten mir nicht bekannt waren.
    Was ist der Vorteil von vector und wie verwendet man es?

    Das ganze herumgepointere ist, weil jede Funktion andere Variablentypen braucht. z.B. substr() funktioniert nur bei strings - soweit logisch. Dateien hingegen kann man nur öffnen wenn der Dateiname als char* oder const char* vorliegt.

    @SeppJ Wie würde denn die Fehlerbehandlung bei Öffnung richtigerweise aussehen?
    Die cout's sind nur Relikte meiner Fehlersuche und werden wieder entfernt sobald ich das Problem beseitigt habe.
    Die merkwürdig speziefische Anzahl von auszulesenden Werten rührt daher, dass alle Dateien, die diese Funktion öffnen können soll von der oben beschrieben Art sind.



  • Mathemalsky schrieb:

    ...

    Dateien hingegen kann man nur öffnen wenn der Dateiname als char* oder const char* vorliegt.

    Vielleicht den Compiler auf C++11 oder C++14 einstellen, dann kannst du auch mit Strings arbeiten und hast weniger C-Relikte.

    MfG


  • Mod

    Mathemalsky schrieb:

    @SeppJ Wie würde denn die Fehlerbehandlung bei Öffnurichtigerweise aussehen?

    Du gibst eine Meldung aus, aber dann liest du doch aus der Datei (oder versuchst es zumindest) und verarbeitest dann die (nicht gelesenen) Daten. Kommt dir das sinnvoll vor?



  • Mathemalsky schrieb:

    Ok könntest du mir genauer beschreiben, wie das mit -fsanitize=address geht? Ist das ein Parameter?
    Momentan kompillere ich mit g++ *.cpp -o test

    Ja, kompiliere mit:
    g++ -fsanitize=address -Wall -Wextra -std=c++14 -g test.cpp

    Also statt des std=c++14 kannst du auch c++11 nehmen oder es weglassen. Ich würde dir aber vorschlagen, den neuesten Standard zu nehmen, den dein Compiler kennt (vielleicht auch sogar schon c++17?)

    Du führst dein Programm dann ganz normal aus und bekommst im besten Fall eine Fehlermeldung mit Zeile.

    wob schrieb:

    herumgepointere

    Weil ich es so gelernt habe und andere Möglichkeiten mir nicht bekannt waren.
    Was ist der Vorteil von vector und wie verwendet man es?

    Das steht in jedem guten Grundlagenbuch. Vorteil: es ist deutlich einfacher und weniger fehleranfällig. Ich frage mich, wer C++ ohne std::vector unterrichtet - das ist so ungefähr die wichtigste Klasse, die es in C++ gibt.



  • Danke für eure Tipps 🙂
    Ich habe die Funktion entsprechend umgestaltet.
    Die Funktion access_value funktioniert nun und sieht so aus:

    void access_value(std::vector<int> &value, std::string Filename)
    {
    	const char* new_filename = Filename.c_str();	
    	std::ifstream myfile;								// opens a filestream
    	myfile.open(new_filename, std::ios_base::in);		// opens a file
    	bool Error = Error02(new_filename, 1);				// proof the file exists
    	if (Error == 0);
    	for (int i = 0; i < 95; i++)						// for every line in the file
    	{
    		myfile >> value[i];								// copies line i from the file to entry i in the vector
    	}
    	myfile.close();										// closes the file
    }
    

    Auch das Problem in PrintFile ist gelöst.
    Wie ich den Compiler auf c++11 oder 14 umstelle habe ich noch nicht herausgefunden, aber das werde ich schon noch und so funktioniert es erstmal.

    Danke nochmal allen, die geantwortet haben. 😉



  • Noch ein paar Anmerkungen:

    void access_value(std::vector<int> &value, const std::string& Filename) // auch Filename als const string&
    {
        const char* new_filename = Filename.c_str();   
        std::ifstream myfile( new_filename ); // Immer schön den Konstruktor nutzen. Das i in ifstream steht für in, std::ios_base::in ist unnötig.
        bool Error = Error02(new_filename, 1);
        if (Error == 0) // Immer schön {} zur if dazu machen. Und bloß kein ; machen.
    	{
    		for (int i = 0; i != 95; ++i) // Gut, dass man es mal gehört hat: != statt < und Postfix (natürlich ist deins auch OK)
    		{
    			myfile >> value[i]; // Was ist wenn ein Fehler passiert???
    		}
    	}
        //myfile.close(); Unnötig, denn es gibt einen tollen Destruktor, der macht das schon. google mal nach RAII.
    }
    


  • Wie ich den Compiler auf c++11 oder 14 umstelle habe ich noch nicht herausgefunden,

    Hatte ich doch geschrieben: mit der Option -std=c++11 bzw. -std=c++14.

    Aber noch eine ganz andere Sache: wo kommt die 95 in der Loop her?

    for (int i = 0; i < 95; ++i)
    

    Ich habe 2 Alternativen:
    1. Man kennt die Zahl und der vector hat schon die Länge:

    for (int i = 0; i < value.size(); ++i)
        myfile >> value[i];
    

    (wobei ich da noch kritisieren würde, dass die Variable "value" statt "values" heißt - und dass das Fehlerchecking fehlt)

    2. Man liest solange so viel ein, wie vorhanden ist:

    std::vector<int> access_value(const std::string& Filename) {
        std::vector<int> result;
        const char* new_filename = Filename.c_str(); 
        std::ifstream myfile( new_filename );
        bool Error = Error02(new_filename, 1);
        if (Error == 0) {
            std::copy(std::istream_iterator<int>(myfile),
                      std::istream_iterator<int>(),
                      std::back_inserter(result));
            // add error checking
        }
        return result;
    }
    


  • out schrieb:

    Noch ein paar Anmerkungen:

    std::ifstream myfile( new_filename ); // Immer schön den Konstruktor nutzen.
    

    Geht das äquivalent dazu auch in der Ausgabe mit:

    std::ofstream myfile( new_filename );
    

    ?

    wob schrieb:

    Aber noch eine ganz andere Sache: wo kommt die 95 in der Loop her?

    Die 95 ist in dem Programm eine Konstante, die daher kommt, dass es für in der Eingabe alle ASCII Zeichen von 32 bis 126 zugelassen sind (95 Stück) für jedes Zeichen gibt es einen Wert. Variabel ist nur, woher die Werte eingelesen werden sollen. Deshalb fand ich es in diesem Fall übersichtlicher die 95 konkret hinzuschreiben.

    wob schrieb:

    Hatte ich doch geschrieben: mit der Option -std=c++11 bzw. -std=c++14.

    Entschuldige ich habe die Nachricht in der das Stand - warum auch immer - erst jetzt gesehen.

    wob schrieb:

    Ich frage mich, wer C++ ohne std::vector unterrichtet - das ist so ungefähr die wichtigste Klasse, die es in C++ gibt.

    Ich kenne die Gründe dafür auch nicht. Es wird in c++ programmiert, aber manchmal gibt es Vorgaben, was über include eingebunden werden darf. In der Regel sind das <iostream>, <ctime>, <fstream> und <string>.
    Vielleicht um es für Einsteiger übersichtlich zu halten oder den Korrekturaufwand zu mindern.
    Da ich dieses Programm jedoch für den Privatgebrauch schreibe können mir die Restriktionen diesmal egal sein.



  • Mathemalsky schrieb:

    out schrieb:

    Noch ein paar Anmerkungen:

    std::ifstream myfile( new_filename ); // Immer schön den Konstruktor nutzen.
    

    Geht das äquvalent dazu auch in der Ausgabe mit:

    std::ofstream myfile( new_filename );
    

    ?

    Ja.


Log in to reply