开发者

C++ Pointer Assignment in custom class constructor

So I have a custom class 'Book' that has a bunch of member variables, amongst which are a vector of another custom class called 'Review' and a pointer to point at that vector as I need to pass that around through function calls in a driver program. The driver program reads the details of the each book (such as title, author, publish date etc.) from a text file and inserts into a temporary 'Book' object which it then adds onto a vector of Books maintained by the driver program. Here is the code for reading from the file:

ifstream file("books.txt");
string line;
if(file.is_open())
{
    while(!file.eof())
    {
        Book buffBook;
        getline(file, line);
        buffBook.setTitle(line);
        getline(file, line);
        buffBook.setAuthor(line);
        getline(file, line);
        buffBook.setPubDate(line);
        getline(file, line);
        buffBook.setIsbn(line);
        getline(file, line);
        buffBook.setCategory(line);
        getline(file, line);
        buffBook.setFormat(line);
        getline(file, line);
        buffBook.setSynopsis(line);
        vectBooks.push_back(buffBook);
    }
}

else
    cout<<"File not found(1)!"<<endl;

file.close();

This is running inside a int main() function.

One of the functions of the driver program is to add a review, which takes in data from the user and inserts it into a temporary 'Review' object. That object is then passed onto to be inserted in to the vector of reviews for the corresponding book. Here is the code for the addReview() function:

void addReview()
{
    string name = "";
    string title;
    Book rTemp;
    cin.ignore();
    cout<<"Which book would you like to rate (Title)?: ";
    getline(cin, name);
    name = toLow(name);
    Review r;
    string re, user;
    int ra;
    cout<<"Username (Full Name): ";
    getline(cin, user);
    string fname = user.substr(0, user.find_first_of(' '));
    string lname = user.substr( user.find_first_of(' ') + 1, user.size());
    r.setUsrFName(fname);
    r.setUsrLName(lname);
    cout<<"Enter rating (1-5):";
    cin>>ra;
    r.setRating(ra);
    cout<<"Enter a short textual review: ";
    cin.ignore();
    getline(cin, re);
    r.setReview(re);
    for(unsigned int i = 0; i < vectBooks.size(); i++)
    {
         title = toLow(vectBooks[i].getTitle());
         if(title.find(name) != string::npos)
         {
             vectBooks[i].getReviews()->push_back(r);
         }
    }
}

Now the problem is if i add a review, it adds it for all the books. In other words, when I fetch the book info for any book, the review shows on all the books. I assume this is a problem with the pointer as it seems like all the reviews are getting stored in the same vector. I am not sure where I am messing up but I have a feeling it's with the pointer some where. Any help is appreciated.

Thank You

UPDATE

The point to the title of this problem is that I am doing the assignment of the pointer to the vector of Reviews in the constructor of the Book class, of which those 2 are member variables. Code as follows for constructor:

Book::Book()
{
    pointRev = &vectReviews;
}

UPDATE 2

Here is the code for the Book Class and supporting classes:

book.h

#ifndef BOOK_H_
#define BOOK_H_

#include <string>
#include <iostream>
#include <vector>
#include "review.h"

using namespace std;

class Book
{
private:
    string title;
    string author;
    string pubDate;
    string isbn;
    string category;
    string format;
    string synopsis;
    vector<Review> vectReviews;
    vector<Review>* pointRev;
public:
    Book::Book() : pointRev(&vectReviews) {};
    string getAuthor() const;
    string getCategory() const;
    string getFormat() const;
    string getIsbn() const;
    string getPubDate() const;
    string getSynopsis() const;
    string getTitle() const;
    vector<Review>* getReviews();
    void setAuthor(string author);
    void setCategory(string category);
    void setFormat(string format);
    void setIsbn(string isbn);
    void setPubDate(string pubDate);
    void setSynopsis(string synopsis);
    void setTitle(string title);

    friend ostream& operator <<(ostream& out, Book& book);
    vector<Review> *getPointRev() const;
    vector<Review> getVectReviews() const;
    void setPointRev(vector<Review> *pointRev);
    void setVectReviews(vector<Review> vectReviews);


};

#endif /* BOOK_H_ */

book. cpp

#include "book.h"
string Book::getAuthor() const
{
    return author;
}

string Book::getCategory() const
{
    return category;
}

string Book::getFormat() const
{
    return format;
}

string Book::getIsbn() const
{
    return isbn;
}

string Book::getPubDate() const
{
    return pubDate;
}

string Book::getSynopsis() const
{
    return synopsis;
}

string Book::getTitle() const
{
    return title;
}

void Book::setAuthor(string author)
{
    this->author = author;
}

void Book::setCategory(string category)
{
    this->category = category;
}

void Book::setFormat(string format)
{
    this->format = format;
}

void Book::setIsbn(string isbn)
{
    this->isbn = isbn;
}

void Book::setPubDate(string pubDate)
{
    this->pubDate = pubDate;
}

void Book::setSynopsis(string synopsis)
{
    this->synopsis = synopsis;
}

void Book::setTitle(string title)
{
    this->title = title;
}

vector<Review> *Book::getPointRev() const
{
    return pointRev;
}

vector<Review> Book::getVectReviews() const
{
    return vectReviews;
}

void Book::setPointRev(vector<Review> *pointRev)
{
    this->pointRev = pointRev;
}

void Book::setVectReviews(vector<Review> vectReviews)
{
    this->vectReviews = vectReviews;
}

vector<Review>* Book::getReviews()
{
    return pointRev;
}


o开发者_开发技巧stream& operator <<(ostream& out, Book& book)
{
    out<<"\nTitle: "<<book.getTitle()<<endl;
    out<<"Author: "<<book.getAuthor()<<endl;
    out<<"Publish Date: "<<book.getPubDate()<<endl;
    out<<"ISBN: "<<book.getIsbn()<<endl;
    out<<"Category: "<<book.getCategory()<<endl;
    out<<"Format: "<<book.getFormat()<<endl;
    out<<"Synopsis: "<<book.getSynopsis()<<endl;
    cout<<"\n--- Reviews ---"<<endl;
//  vector<Review>* revs = book.getReviews();
    for(unsigned int h = 0; h < book.getReviews()->size(); h++)
    {
        cout<<"Review by: "<<book.getReviews()->at(h).getUsrFName()<<" "<<book.getReviews()->at(h).getUsrLName()<<endl;
        cout<<"Rating: "<<book.getReviews()->at(h).getRating()<<endl;
        cout<<"Review: "<<book.getReviews()->at(h).getReview()<<endl;
    }

    return out;
}

review.h

#ifndef REVIEW_H_
#define REVIEW_H_

#include <string>

using namespace std;

class Review
{
private:
    int rating;
    string review;
    string usrFName;
    string usrLName;
public:
    int getRating() const;
    string getReview() const;
    void setRating(int rating);
    void setReview(string review);
    string getUsrFName() const;
    string getUsrLName() const;
    void setUsrFName(string usrFName);
    void setUsrLName(string usrLName);
};

#endif /* REVIEW_H_ */

review.cpp

#include "review.h"



int Review::getRating() const
{
    return rating;
}

string Review::getReview() const
{
    return review;
}


void Review::setRating(int rating)
{
    this->rating = rating;
}

string Review::getUsrFName() const
{
    return usrFName;
}

string Review::getUsrLName() const
{
    return usrLName;
}

void Review::setUsrFName(string usrFName)
{
    this->usrFName = usrFName;
}

void Review::setUsrLName(string usrLName)
{
    this->usrLName = usrLName;
}

void Review::setReview(string review)
{
    this->review = review;
}


From the behavior you describe, a copy-constructor is running and making two objects that point to the same vector. push_back does indeed use a copy-constructor.

But your first code snippet doesn't make a bunch of copies of the same Book, but a new Book is created on each loop iteration (and then copied into vectBooks.

If Book doesn't have a correct user-defined copy-constructor then you aren't managing pointRev correctly. From the observed behavior, I believe that you have a destructor which frees pointRev, and then the copy inside vectBooks is left with a dangling pointer. Everything after this falls into the category of undefined behavior according to the standard, meaning "anything can happen" Then the next Book happens to reuse the same memory area, so all the instances of Book end up with the same value in the wild pointer. And then updating any one changes the vector (which isn't even alive anymore) seen by all Book instances.

Why are you using a pointer to a std::vector anyway? Much better to just put the vector into the class as a direct data member, which lets the compiler construct, copy, and destruct it automatically with no additional help from you.


Of course, it's entirely possible that you've made vectReviews a global variable, and then every single book points to the same instance. That would a review show up in all books simultaneously, because they share the vector you're adding it to.

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜