Passing istream into a function

Question

I am making a game-type program similar to the idea of pokemon. We have a tournament class that keeps track of several teams(its own class) which consists of pets(its own class) with different kinds of pets being subclasses of CPet.

We are attempting to pass a file name into main, from main pass that file name into the Tournament class. In the Tournament class we open the file with:

 14 //Construct a tournament
 15 CTournament::CTournament(const char *Filename){
 16         //opening file
 17         ifstream inFile(Filename, ios::in);
 18         if(inFile.bad()){
 19                 cout << "File error" << endl;
 20                 return ;
 21         }
 22          //get Teamlist for tournament
 23          while(!(inFile.eof())){
 24                  CTeam* temp = new CTeam;
 25                  temp->ParseTeam(inFile);
 26 
 27                  TeamList.push_back(temp);
 28          }
 29 }

Here we pass inFile into CTeam.ParseTeam which looks like:

     30 void CTeam::ParseTeam(std::istream in){
     31   string readline;
     32   getline(in, readline);
     33   this->TeamName = readline;
     34   while(!(in.eof())&&(readline != " " || readline != "/n"))
     35   {
     36           getline(in, readline);
     37           this->Parse(readline);
     38   }
     39 }

and we are getting the error:

In file included from /usr/include/c++/4.4/ios:39,
                 from /usr/include/c++/4.4/ostream:40,
                 from /usr/include/c++/4.4/iostream:40,
                 from CTournament.h:11,
                 from CTournament.cpp:8:
/usr/include/c++/4.4/bits/ios_base.h: In copy constructor 'std::basic_ios<char, std::char_traits<char> >::basic_ios(const std::basic_ios<char, std::char_traits<char> >&)':
/usr/include/c++/4.4/bits/ios_base.h:790: error: 'std::ios_base::ios_base(const std::ios_base&)' is private
/usr/include/c++/4.4/iosfwd:47: error: within this context
/usr/include/c++/4.4/iosfwd: In copy constructor 'std::basic_istream<char, std::char_traits<char> >::basic_istream(const std::basic_istream<char, std::char_traits<char> >&)':
/usr/include/c++/4.4/iosfwd:53: note: synthesized method 'std::basic_ios<char, std::char_traits<char> >::basic_ios(const std::basic_ios<char, std::char_traits<char> >&)' first required here 
CTournament.cpp: In constructor 'CTournament::CTournament(const char*)':
CTournament.cpp:25: note: synthesized method 'std::basic_istream<char, std::char_traits<char> >::basic_istream(const std::basic_istream<char, std::char_traits<char> >&)' first required here 
CTournament.cpp:25: error:   initializing argument 1 of 'void CTeam::ParseTeam(std::istream)'

I know there is a similar question about this where he did not include fstream. We have included it in both header files.

I thought maybe it was a problem of not passing the correct type into PraseTeam, but I could not find anything very specific on how else to pass the file into ParseTeam to verify whether I was doing it correctly or not.

Thanks in advance.

Solution 1

You need to pass the stream by reference, because streams are not generally copyable:

void CTeam::ParseTeam(std::istream &in)

Solution 2

As @dasblinkenlight already pointed out, you need/want to pass streams by reference.

That's just the tip of the iceberg though. Your code has a number of more pernicious problems. Chief among them is code like:

while(!(inFile.eof()))

Code like this is broken. It won't work. It never has and it never will (short of some major miracle occurring). It's barely possible to put a condition inside the loop (with a break statement) to get out of the loop at the correct time -- but you haven't done so, and almost nobody else does either, and when you do, you might as well turn the loop itself into while (true), because it'll always be that other logic that exits the loop at the right time -- because this loop condition can't and won't. [I don't mean to sound harsh or nasty there, just trying to be completely clear that this code absolutely does not work.]

You have the same basic problem again here:

 while(!(in.eof())&&(readline != " " || readline != "/n"))

This adds what I'd guess is another (though less common and much easier to fix) problem -- your "/n" was almost certainly intended to be "\n".

In most cases, what you really want/need to do is read something from the stream, and have the function that does the reading return a reference to the stream. This will let you read items until the reading fails (at which point the stream's failbit should be set). Once you've done that, you can make the loop conditional on the reading succeeding. In most cases, it's convenient to name that function operator>>, so reading an object of your class uses the same syntax as reading things like ints.

For example, let's take a look at your parseTeam:

 void CTeam::ParseTeam(std::istream in){
   string readline;
   getline(in, readline);
   this->TeamName = readline;
   while(!(in.eof())&&(readline != " " || readline != "/n"))
   {
           getline(in, readline);
           this->Parse(readline);
   }
 }

If it were up to me, I'd write something more like this:

std::istream &operator>>(std::istream &is, CTeam &t) { 
     std::getline(is, t.Teamname);
     CTeam::member member;
     while (is >> member)
         t.members.push_back(member);
     return is;
}

Then, CTeam would have something along this general line:

class CTeam {
    // ...
public:
    class member { 
        // ...     
        friend std::istream &operator>>(std::istream &is, member &m) { 
            // code to read one team member from is
        }
    };
};

I should also add that without knowing more about the program, that organization definitely is not cast in concrete -- for example, it's not clear whether the entity I've shown as CTeam::member might actually be able to exist outside the context of a team. If it can, then you (almost certainly) want to make it an independent class of its own, outside of CTeam. For the moment, I'm simply guessing at how to organize the code based on what I can glean from the code you've posted.

Summary: Anything of the form while (!whatever.eof()) is irretrievably broken. You nearly always want while (read_something()) instead. The convention in C++ is to use >> to read an item from a stream. Follow that convention when/if possible. Oh, and yeah, you pretty much always want to pass streams by reference (the exception being the odd time you need to work with a pointer to a stream instead -- unusual, but it does happen now and again).