ifstream als member

Hallo, ich verstehe absolut nicht wo der Fehler folgenden Programms ist. Könnt Ihr mir genau sagen warum mein gnu compiler (< C++11) schreit?

// main.cpp
#include <iostream>
#include "FileReader.h"

int main(int argc, char** argv) {
   FileReader fr = FileReader();
   fr.openFile("/home/mat/threads/count/test.txt");
   fr.printFile();
   return 0;
}

// ======================= FILE_READER.h
#ifndef _FILE_READER_H_
#define _FILE_READER_H_

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

class FileReader
{
  public:
    FileReader();
    ~FileReader();

    void openFile(char*);
    void printFile();

  private:
    std::ifstream *myfile;
};

#endif

// ============================= FileReader.cpp

#include "FileReader.h"
#include <stdexcept>

FileReader::FileReader()
{
   myfile = new std::ifstream;
}
FileReader::~FileReader()
{
   delete myfile;
}

void FileReader::openFile(char* fileToOpen)
{
   myfile->open(fileToOpen, std::ifstream::in);
//      throw std::runtime_error("File could not be opened!");
}

void FileReader::printFile() {
    std::string line;
    std::cout << "Output of file:" << std::endl;
    while(std::getline(&myfile,line))
    {
        std::cout << line << std::endl;
    }
}

mit der CompilerMeldung:

g++ -o out FileReader.cpp main.cpp
FileReader.cpp: In Elementfunktion »void FileReader::printFile()«:
FileReader.cpp:28:36: Fehler: keine passende Funktion für Aufruf von »getline(std::ifstream**, std::string&)«
FileReader.cpp:28:36: Anmerkung: Kandidaten sind:
/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/bits/basic_string.tcc:1070:5: Anmerkung: template<class _CharT, class _Traits, class _Alloc> std::basic_istream<_CharT, _Traits>& std::getline(std::basic_istream<_CharT, _Traits>&, std::basic_string<_CharT, _Traits, _Alloc>&, _CharT)
/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/bits/basic_string.h:2734:5: Anmerkung: template<class _CharT, class _Traits, class _Alloc> std::basic_istream<_CharT, _Traits>& std::getline(std::basic_istream<_CharT, _Traits>&, std::basic_string<_CharT, _Traits, _Alloc>&)
main.cpp: In Funktion »int main(int, char**)«:
main.cpp:14:50: Warnung: veraltete Konvertierung von Zeichenkettenkonstante in »char*« [-Wwrite-strings]

übrigens mit

void FileReader::printFile()
{
    std::string line;
    std::cout << "Output of file:" << std::endl;

    while(std::getline(myfile,line))
    {
        std::cout << line << std::endl;
    }
}

beklagt er sich auch mit

g++ -o out FileReader.cpp main.cpp
FileReader.cpp: In Elementfunktion »void FileReader::printFile()«:
FileReader.cpp:28:35: Fehler: keine passende Funktion für Aufruf von »getline(std::ifstream*&, std::string&)«
FileReader.cpp:28:35: Anmerkung: Kandidaten sind:
/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/bits/basic_string.tcc:1070:5: Anmerkung: template<class _CharT, class _Traits, class _Alloc> std::basic_istream<_CharT, _Traits>& std::getline(std::basic_istream<_CharT, _Traits>&, std::basic_string<_CharT, _Traits, _Alloc>&, _CharT)
/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/bits/basic_string.h:2734:5: Anmerkung: template<class _CharT, class _Traits, class _Alloc> std::basic_istream<_CharT, _Traits>& std::getline(std::basic_istream<_CharT, _Traits>&, std::basic_string<_CharT, _Traits, _Alloc>&)
main.cpp: In Funktion »int main(int, char**)«:
main.cpp:14:50: Warnung: veraltete Konvertierung von Zeichenkettenkonstante in »char*« [-Wwrite-strings

std::ifstream *myfile;

->

std::ifstream myfile;

daddy_felix

Schau dir doch nochmal an, was std::getline für Parameter erwartet, dann solltest du dein Problem recht schnell lösen können.

Kleiner Tipp: es ist weder ein Pointer, noch ein "Pointer auf Pointer"

Skym0sh0

void FileReader::printFile()
{
    std::string line;
    std::cout << "Output of file:" << std::endl;

    while(std::getline(*myfile,line))
    {
        std::cout << line << std::endl;
    }
}

So wärs richtig.

Wenn du den fstream-member nicht als Pointer machst, dann wirst du deine Klassen nicht kopieren könne.Was deine erste Zeile in der main falsch macht.

Skym0sh0 schrieb:

Wenn du den fstream-member nicht als Pointer machst, dann wirst du deine Klassen nicht kopieren könne.Was deine erste Zeile in der main falsch macht.

Wenn er den fstream-member als Pointer lässt, wird er die Regel der grossen 3 verletzen, was seine erste Zeile in der main falsch macht.

so wie ich das verstehe:
getline erwartet eine Referenz auf einen input-stream.

ich kann über

void FileReader::printFile()
{
    std::string line;
    std::cout << "Output of file:" << std::endl;
    while(std::getline(*myfile,line))
    {
        std::cout << line << std::endl;
    }
}

den code zum Laufen bringen, verstehe aber nicht warum. Ich dereferenziere myfile und übergebe doch somit an getline ein ifstream-objekt oder?

Übrigens der vorschlag von "remove pointer out of c++" funktioniert so einfach nicht. Der Compiler beklagt sich dann mit:

g++ -o out FileReader.cpp main.cpp
In file included from /usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/ios:45:0,
                 from /usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/ostream:40,
                 from /usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/iostream:40,
                 from main.cpp:1:
/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/bits/ios_base.h: In Copy-Konstruktor »std::basic_ios<char>::basic_ios(const std::basic_ios<char>&)«:
/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/bits/ios_base.h:788:5: Fehler: »std::ios_base::ios_base(const std::ios_base&)« ist privat
/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/bits/basic_ios.h:64:11: Fehler: in diesem Zusammenhang
In file included from FileReader.h:4:0,
                 from main.cpp:2:
/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/fstream: In Copy-Konstruktor »std::basic_ifstream<char>::basic_ifstream(const std::basic_ifstream<char>&)«:
/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/fstream:420:11: Anmerkung: erzeugte Methode »std::basic_ios<char>::basic_ios(const std::basic_ios<char>&)« zuerst hier erfordert 
/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/streambuf: In Copy-Konstruktor »std::basic_filebuf<char>::basic_filebuf(const std::basic_filebuf<char>&)«:
/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/streambuf:782:7: Fehler: »std::basic_streambuf<_CharT, _Traits>::basic_streambuf(const __streambuf_type&) [mit _CharT = char, _Traits = std::char_traits<char>, std::basic_streambuf<_CharT, _Traits>::__streambuf_type = std::basic_streambuf<char>]« ist privat
/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/fstream:69:11: Fehler: in diesem Zusammenhang
/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/fstream: In Copy-Konstruktor »std::basic_ifstream<char>::basic_ifstream(const std::basic_ifstream<char>&)«:
/usr/lib/gcc/i686-pc-linux-gnu/4.6.3/include/g++-v4/fstream:420:11: Anmerkung: erzeugte Methode »std::basic_filebuf<char>::basic_filebuf(const std::basic_filebuf<char>&)« zuerst hier erfordert 
In file included from main.cpp:2:0:
FileReader.h: In Copy-Konstruktor »FileReader::FileReader(const FileReader&)«:
FileReader.h:9:7: Anmerkung: erzeugte Methode »std::basic_ifstream<char>::basic_ifstream(const std::basic_ifstream<char>&)« zuerst hier erfordert 
main.cpp: In Funktion »int main(int, char**)«:
main.cpp:6:31: Anmerkung: erzeugte Methode »FileReader::FileReader(const FileReader&)« zuerst hier erfordert 
main.cpp:7:50: Warnung: veraltete Konvertierung von Zeichenkettenkonstante in »char*« [-Wwrite-strings]

Wenn er den fstream-member als Pointer lässt, wird er die Regel der grossen 3 verletzen, was seine erste Zeile in der main falsch macht.

was sind die grossen 3?

Skym0sh0

Der große Fehler kommt daher, dass ein Stream nicht kopierbar ist.
Das Argument, dass anders die Regel der großen 3 verletzt wird, lasse ich nicht durchgehen. Man muss die Regel einfach einhalten ;).

Die Regel der großen 3 bedeutet, dass: Wenn du entweder Kopierkonstruktor, Zuweisungsoperator oder Destruktor selsbt definieren musst, dann musst du jeweils auch die anderen beiden selbst schreiben.

Der Compiler z.B. generiert für deine Klasse einen trivialen Kopierkonstruktor, der eine Shallow-Copy macht, willst du nun eine Deep-Copy, dann musst du alle 3 Funktionen selbst schreiben.

SeppJ

Skym0sh0 schrieb:

Der Compiler z.B. generiert für deine Klasse einen trivialen Kopierkonstruktor, der eine Shallow-Copy macht, willst du nun eine Deep-Copy, dann musst du alle 3 Funktionen selbst schreiben.

Was sich aber bei einer Klasse mit ifstream als Member, egal ob über (besitzenden) Pointer oder direkt, nicht sinnvoll definieren lässt. Was sollte denn die Kopie eines Streams sein? Das beantwortet doch schon die Standardbibliothek nicht, weil es darauf keine gute Antwort gibt. Eine streamartige Klasse, wie sie hier vorliegt, sollte daher wohl ebenfalls unkopierbar sein. Das heißt, die sinnvolle Art, den Kopierkonstruktor und Zuweisungsoperator zu definieren, wäre, sie private oder deleted zu machen.

Th69

s. Regel der Großen Drei

Skym0sh0

SeppJ schrieb:

Skym0sh0 schrieb:

Der Compiler z.B. generiert für deine Klasse einen trivialen Kopierkonstruktor, der eine Shallow-Copy macht, willst du nun eine Deep-Copy, dann musst du alle 3 Funktionen selbst schreiben.

Was sich aber bei einer Klasse mit ifstream als Member, egal ob über (besitzenden) Pointer oder direkt, nicht sinnvoll definieren lässt. Was sollte denn die Kopie eines Streams sein? Das beantwortet doch schon die Standardbibliothek nicht, weil es darauf keine gute Antwort gibt. Eine streamartige Klasse, wie sie hier vorliegt, sollte daher wohl ebenfalls unkopierbar sein. Das heißt, die sinnvolle Art, den Kopierkonstruktor und Zuweisungsoperator zu definieren, wäre, sie private oder deleted zu machen.

Das stand aber nicht zur Disposition. Klar macht es keinen Sinn einen Dateizugriff zu kopieren, trotzdem ist das Programm mit dem zeiger erstmal lauffähig.

danke euch schonmal.
ich bin jetzt etwas verwirrt. sehe ich das richtig dass meine erste variante mit *myfile unsauber ist und ich eher std::ifstream myfile; als member benutzen sollte?
Dazu muss ich dann wohl die Regel der großen Drei beachten und Zuweisung+Desktruktor bzw. kopier-konstruktor selbst nachschreiben oder?

sehe ich das richtig dass dann (wenn die regel der großen 3 korrekt implementiert ist) ich bei std::getline(myline,line) einfach benutzen kann ?

daddy_felix

Skym0sh0 schrieb:

Das stand aber nicht zur Disposition. Klar macht es keinen Sinn einen Dateizugriff zu kopieren, trotzdem ist das Programm mit dem zeiger erstmal lauffähig.

"erstmal" ist gut.

FileReader f;
{
FileReader f2 = f;
}
f.UseStreamMember(); // oh oh...

verärgerter schrieb:

und ich eher std::ifstream myfile; als member benutzen sollte?

Gegenfrage: warum willst du überhaupt einen stream als member halten?

Gegenfrage: warum willst du überhaupt einen stream als member halten?

Ich will nur ein wenig üben und wollte ein einfaches programm schreiben was mir die Wörter in einer Datei zählt und deren Vorkommen ausgibt.
Da dachte ich ich schreibe eine einfache Klasse FileReader die einen member als inputstream trägt.

FileReader funktinioniert jetzt, hat aber eben den sream als pointer:

#include "FileReader.h"
#include <stdexcept>
#include <map>

FileReader::FileReader() 
{
   myfile = new std::ifstream;
}
FileReader::~FileReader()
{
   delete myfile; 
}

void FileReader::openFile(char* fileToOpen)
{
   myfile->open(fileToOpen, std::ifstream::in);
}

void FileReader::printFile()
{
    std::string line;
    std::cout << "Output of file:" << std::endl;
    while(std::getline(*myfile,line))
        std::cout << line << std::endl;
}

void FileReader::countWords()
{
   std::map<std::string,unsigned int> words;
   std::string line;
   std::map<std::string,unsigned int>::iterator mapit;

   while(std::getline(*myfile,line))
   {  
      mapit = words.find(line);
      if(mapit != words.end()) mapit->second++;
      else words[line] = 1;
   }

   std::cout << "Starting output of words ..." << std::endl;
   for(mapit = words.begin(); mapit != words.end(); mapit++)
      std::cout << " " << mapit->first << " : " << mapit->second << std::endl;
}

Skym0sh0 schrieb:

Das stand aber nicht zur Disposition. Klar macht es keinen Sinn einen Dateizugriff zu kopieren, trotzdem ist das Programm mit dem zeiger erstmal lauffähig.

Das Programm ist auch ohne Zeiger erstmals lauffähig. Kopieren geht auch mit Zeiger nicht. Ein Zeiger macht NULL Sinn.

Davon abgesehen macht die Klasse auch keinen Sinn.

void printFile(std::istream& in)
{
    std::string line;
    std::cout << "Output of file:" << std::endl;
    while(std::getline(in,line))
        std::cout << line << std::endl;
}

void countWords(std::istream& in)
{
   std::map<std::string,unsigned int> words;
   std::string line;
   std::map<std::string,unsigned int>::iterator mapit;

   while(std::getline(in,line))
   {  
      mapit = words.find(line);
      if(mapit != words.end()) mapit->second++;
      else words[line] = 1;
   }

   std::cout << "Starting output of words ..." << std::endl;
   for(mapit = words.begin(); mapit != words.end(); mapit++)
      std::cout << " " << mapit->first << " : " << mapit->second << std::endl;
}

Das macht genau das, was der FileReader auch kann und sogar besser, denn man ist nicht mehr auf Dateiströme begrenzt. Es herrscht kein OOP-Zwang wenn OOP keinen Sinn macht.