-1

I have an array of dvd from a Video class I created

Video dvd[10];

each video has the property,

class Video {
    string _title;
    string _genre;
    int _available;
    int _holds;
public:
    Video(string title, string genre, int available, int holds);
    Video();
    void print();
    void read(istream & is, Video dvd);
    int holds();
    void restock(int num);
    string getTitle();
    ~Video();
};

I'm trying to fill up this array with data from my text file where each info such as the title and genre is separated by a comma

Legend of the seeker, Fantasy/Adventure, 3, 2
Mindy Project, Comedy, 10, 3
Orange is the new black, Drama/Comedy, 10, 9

I've tried using getline(in, line, ',') but my brain halts when its time to insert each line into the dvd array.

I also created a read method to read each word separated by a whitespace but I figured thats not what I really want.

I also tried to read a line with getline, store the line in a string and split it from there but I get confused along the line.

**I can get the strings I need from each line, my confusion is in how to insert it into my class array in the while loop especially when I can only read one word at a time.

I need help on what approach I should follow to tackle this problem.

**My code

#include <iostream>
#include <fstream>
#include <cassert>
#include <vector>

#define MAX 10

using namespace std;

class Video {
    string _title;
    string _genre;
    int _available;
    int _holds;
public:
    Video(string title, string genre, int available, int holds);
    Video();
    void print();
    void read(istream & is, Video dvd);
    int holds();
    void restock(int num);
    string getTitle();
    ~Video();
};

Video::Video(string title, string genre, int available, int holds){
    _title = title;
    _genre = genre;
    _available = available;
    _holds = holds;
}

void Video::read (istream & is, Video dvd)   
{  
    is >> _title >> _genre >> _available>>_holds;
    dvd = Video(_title,_genre,_available,_holds);

}

int Video::holds(){
    return _holds;
}

void Video::restock(int num){
    _available += 5;
}

string Video::getTitle(){
    return _title;
}

Video::Video(){

}

void Video::print(){
    cout<<"Video title: " <<_title<<"\n"<<
    "Genre: "<<_genre<<"\n"<<
    "Available: " <<_available<<"\n"<<
    "Holds: " <<_holds<<endl;
}

Video::~Video(){
    cout<<"DESTRUCTOR ACTIVATED"<<endl;
}

int main(int params, char **argv){

    string line;
    int index = 0;
    vector<string> tokens;
    //Video dvd = Video("23 Jump Street", "comedy", 10, 3);
    //dvd.print();
    Video dvd[MAX];
    dvd[0].holds();

    ifstream in("input.txt");

    /*while (getline(in, line, ',')) {
        tokens.push_back(line);

    }
    for (int i = 0; i < 40; ++i)
    {
        cout<<tokens[i]<<endl;
    }*/

    if(!in.fail()){
        while (getline(in, line)) {

            dvd[index].read(in, dvd[index]);
            /*cout<<line<<endl;
            token = line;
            while (getline(line, token, ',')){

            }
            cout<<"LINE CUT@@@@@"<<endl;
            cout<<line<<endl;
            cout<<"TOKEN CUT@@@@@"<<endl;*/
            //dvd[index] = 
            index++;
        }
    }else{
        cout<<"Invalid file"<<endl;
    }


    for (int i = 0; i < MAX; ++i)
    {
        dvd[i].print();
    }


}
ekeith
  • 644
  • 1
  • 10
  • 33
  • 1
    Now would be a great time to learn regular expressions. – Alan Stokes May 09 '17 at 22:03
  • Possible duplicate of [How can I read and parse CSV files in C++?](http://stackoverflow.com/questions/1120140/how-can-i-read-and-parse-csv-files-in-c) – Christopher Pisz May 09 '17 at 22:04
  • Also, what purpose would that user-defined destructor for `Video` serve? All of your members in the `Video` class don't need any special handling on destruction. – PaulMcKenzie May 09 '17 at 22:05
  • I've tried the answer on the question. using a string vector to store the values but I can't use it for each instance of the dvd array @ChristopherPisz – ekeith May 09 '17 at 22:07
  • I can get all the strings I need to insert from each line, the confusion is how to insert it into my array. – ekeith May 09 '17 at 22:11
  • @ekeith `int i = 0; ... some loop on i ... dvd[i] = Video(arg1, arg2, arg3, arg4);`. Also, my comment on the destructor is valid. You are potentially turning off the compiler's optimizer when you have a user-defined destructor. Thus it doesn't make sense to stick a destructor there for no reason. – PaulMcKenzie May 09 '17 at 22:13
  • arg1, arg2... can only be read once in the while loop especially if I'm using getline(in, line, ','). The only way I can use that is by in>>title>>.... but that doesn't work for me (its my read method) @PaulMcKenzie – ekeith May 09 '17 at 22:16
  • Please post your code. A loop would just mean read the next line of strings, create a temporary `Video` object just as my comment above does, and assign it to `dvd[i]`, I don't see the issue here. – PaulMcKenzie May 09 '17 at 22:23
  • Okay, I have updated it @PaulMcKenzie – ekeith May 09 '17 at 22:27
  • The code you have currently is passing the entire stream to `read` rather than the line you obtained. Take that line and then tokenize it by whitespace or commas. – Christopher Pisz May 09 '17 at 22:30
  • Off topic: Far as I can see here, you aren't breaking the rules, but be warned: underscore prefixes in C++ mean something. Forewarned is forearmed, so [What are the rules about using an underscore in a C++ identifier?](http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier) – user4581301 May 09 '17 at 22:31
  • @ekeith Right now you are independently trying to read a line, and then read it again for each Video. The first time you've read the line, that's it, that line is no longer available to "read again". So either you read that line once and parse that line using string functions (or a subsequent `std::istringstream`), or `Video` itself reads that line (no outside "getline" call) and figures out what to do with it. Two reads as you're doing now is totally wrong. – PaulMcKenzie May 10 '17 at 00:06

1 Answers1

1

First, I would change the Video::read function into an overload of operator >>. This will allow the Video class to be used as simply as any other type when an input stream is being used.

Also, the way you implemented read as a non-static member function returning a void is not intuitive and very clunky to use. How would you write the loop, and at the same time detect that you've reached the end of file (imagine if there are only 3 items to read -- how would you know to not try to read a fourth item)? The better, intuitive, and frankly, de-facto way to do this in C++ is to overload the >> operator.

(At the end, I show how to write a read function that uses the overloaded >>)

class Video
{
   //...
   public:
      friend std::istream& operator >> (std::istream& is, Video& vid);
   //..
};

I won't go over why this should be a friend function, as that can be easily researched here on how to overload >>.

So we need to implement this function. Here is an implementation that reads in a single line, and copies the information to the passed-in vid:

std::istream& operator >> (std::istream& is, Video& vid)
{
    std::string line;
    std::string theTitle, theGenre, theAvail, theHolds;

    // First, we read the entire line
    if (std::getline(is, line))
    {
        // Now we copy the line into a string stream and break 
        // down the individual items
        std::istringstream iss(line);

        // first item is the title, genre, available, and holds
        std::getline(iss, theTitle, ',');
        std::getline(iss, theGenre, ',');
        std::getline(iss, theAvail, ',');
        std::getline(iss, theHolds, ',');

       // now we can create a Video and copy it to vid
       vid = Video(theTitle, theGenre, 
                   std::stoi(theAvail), // need to change to integer
                   std::stoi(theHolds)); // same here
    }
    return is;  // return the input stream
}

Note how vid is a reference parameter, not passed by value. Your read function, if you were to keep it, would need to make the same change.

What we did above is that we read the entire line in first using the "outer" call to std::getline. Once we have the line as a string, we break down that string by using an std::istringstream and delimiting each item on the comma using an "inner" set of getline calls that works on the istringstream. Then we simply create a temporary Video from the information we retrieved from the istringstream and copy it to vid.

Here is a main function that now reads into a maximum of 10 items:

int main()
{
    Video dvd[10];
    int i = 0;
    while (i < 10 && std::cin >> dvd[i])
    {
        dvd[i].print();
        ++i;
    }
}

So if you look at the loop, all we did is 1) make sure we don't go over 10 items, and 2) just use cin >> dvd[i], which looks just like your everyday usage of >> when inputting an item. This is the magic of the overloaded >> for Video.

Here is a live example, using your data.


If you plan to keep the read function, then it would be easier if you changed the return type to bool that returns true if the item was read or false otherwise, and just calls the operator >>.

Here is an example:

bool Video::read(std::istream & is, Video& dvd)
{
    if (is.good())
    {
        is >> dvd;
        return true;
    }
    return false;
}

And here is the main function:

int main()
{
    Video dvd[10];
    int i = 0;
    while (i < 10 && dvd[i].read(std::cin, dvd[i]))
    {
        dvd[i].print();
        ++i;
    }
}

Live Example #2

However, I still say that the making of Video::read a non-static member makes the code in main clunky.

PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45