0

I have a program that reads the content from a file and creates objects.
I store these objects into a vector<Component *>. So I can call a virtual function on each object.
The virtual function does not work. Any other class function works fine on the vector objects.
Does anybody know what is going on?

void upload(vector<Component*>);
vector<Component*>download();

int main() {
    vector<Component*>vec = download();

    vec[0]->printSpecs(); // read acces violation

    // Write all products from vector to file (overwrite)
}

void upload(vector<Component*> x) {
    // write each class object to specific file
    ofstream f;
    for (int i = 0; i < x.size(); i++) {
        if (x[i]->rank() == 1) {
            f.open("CPU.txt", ios::binary);
            f.write(reinterpret_cast<char*>(x[i]), sizeof(CPU));
            f.close();
        }
        if (x[i]->rank() == 2) {
            f.open("Case.txt", ios::binary);
            f.write(reinterpret_cast<char*>(x[i]), sizeof(Case));
            f.close();
        }
        if (x[i]->rank() == 3) {
            f.open("PS.txt", ios::binary);
            f.write(reinterpret_cast<char*>(x[i]), sizeof(PS));
            f.close();
        }
        if (x[i]->rank() == 4) {
            f.open("GPU.txt", ios::binary);
            f.write(reinterpret_cast<char*>(x[i]), sizeof(GPU));
            f.close();
        }
        if (x[i]->rank() == 5) {
            f.open("HD.txt", ios::binary);
            f.write(reinterpret_cast<char*>(x[i]), sizeof(HD));
            f.close();
        }
        if (x[i]->rank() == 6) {
            f.open("RAM.txt", ios::binary);
            f.write(reinterpret_cast<char*>(x[i]), sizeof(RAM));
            f.close();
        }
        if (x[i]->rank() == 7) {
            f.open("SSD.txt", ios::binary);
            f.write(reinterpret_cast<char*>(x[i]), sizeof(SSD));
            f.close();
        }
    }
}

vector<Component*> download() {
    vector<Component*>items;
    ifstream f;

    // read cpu objects into vector
    f.open("CPU.txt", ios::binary);
    while (!f.eof()) {
        static CPU temp;
        f.read(reinterpret_cast<char*>(&temp), sizeof(CPU));
        Component* x = &temp;
        items.push_back(x);
    }
    items.pop_back();
    f.close();
    // read GPU objects into vector

    f.open("GPU.txt", ios::binary);
    while (!f.eof()) {
        static GPU temp;
        f.read(reinterpret_cast<char*>(&temp), sizeof(GPU));
        Component* x = &temp;
        items.push_back(x);
    }
    items.pop_back();
    f.close();

    // read HD objects into vector

    f.open("HD.txt", ios::binary);
    while (!f.eof()) {
        static HD temp;
        f.read(reinterpret_cast<char*>(&temp), sizeof(HD));
        Component* x = &temp;
        items.push_back(x);
    }
    items.pop_back();
    f.close();

    // read PS objects into vector

    f.open("PS.txt", ios::binary);
    while (!f.eof()) {
        static PS temp;
        f.read(reinterpret_cast<char*>(&temp), sizeof(PS));
        Component* x = &temp;
        items.push_back(x);
    }
    items.pop_back();
    f.close();
    // read SSD objects into vector

    f.open("SSD.txt", ios::binary);
    while (!f.eof()) {
        static SSD temp;
        f.read(reinterpret_cast<char*>(&temp), sizeof(SSD));
        Component* x = &temp;
        items.push_back(x);
    }
        // read RAM objects into vector

    items.pop_back();
    f.close();
    f.open("RAM.txt", ios::binary);
    while (!f.eof()) {
        static RAM temp;
        f.read(reinterpret_cast<char*>(&temp), sizeof(RAM));
        Component* x = &temp;
        items.push_back(x);
    }
    items.pop_back();
    f.close();

    // read case objects into vector

    f.open("Case.txt", ios::binary);
    while (!f.eof()) {
        static Case temp;
        f.read(reinterpret_cast<char*>(&temp), sizeof(Case));
        Component* x = &temp;
        items.push_back(x);
    }
    items.pop_back();
    f.close();

    return items;
}

#ifndef COMPONENT_H
#define COMPONENT_H
#define SIZE 50


class Component {
protected:

    int stock;
    float price;
    char name[SIZE];
    char manufacturer[SIZE];
    bool laptop;
public:
    Component();
    ~Component();
    void setManufacturer(const char*);
    void setName(const char *);
    void setPrice(float);
    void setStock(int);
    void setLaptop(bool);
    virtual void printSpecs() = 0; // the pure virtual function
    const char * getManufacturer() ;
    const char * getName() ;
    float getPrice() const;
    int getStock() const;
    bool getLaptop() const;
    virtual int rank() = 0;
};

#endif


// One of the derived classes

void CPU::printSpecs() {

    cout << "The specifications of this CPU are:\n"
        << "Name: " << getName() << endl
        << "Price: EUR " << price << endl
        << "Laptop: " << ((laptop == true) ? "yes" : "no" )<< endl
        << "Stock: " << stock << endl
        << "Manufacturer: " << getManufacturer() << endl
        << "Speed: " << speed << "GHz" << endl
        << "Cores: " << cores << endl;
}
Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • the code looks alright but you seem to be inserting the same static variable. instead of `Component* x = &temp` try `Component* x = new CPU(temp)` etc. –  Dec 14 '19 at 16:03

2 Answers2

2

There are multiple fundamental bugs in the shown code. These fundamental bugs are repeated multiple times, this is just one occurence:

    while (!f.eof()) {
        static CPU temp;
        f.read(reinterpret_cast<char*>(&temp), sizeof(CPU));
        Component* x = &temp;
        items.push_back(x);
    }

There are at least three fatal bugs here.

1) while (!f.eof()) is always a bug when it's used like this.

2) On each iteration of this loop, this overwrites the static temp variable, and every time the same exact pointer to the same exact object gets pushed into the vector. In the end, you wind up with multiple copies of the same pointer, to the same object in the vector, that contains the last record read from the file.

3) Except that it doesn't really have the "last record read from the file" anyway, because even that part is fundamentally broken. It is clear from the context that the CPU is not a POD. Although your code fails to meet the requirements for a minimal reproducible example, it is clear from your description that it has at least one virtual function. Which means that its contents cannot just be read or written from files, as binary blobs like this. C++ does not work this way. This can only be done with classes that are PODs, and this one isn't.

To fix the shown code you must fix at least three different things.

1) Properly check for the end of the file being read.

2) Don't overwrite the same object, each time, and add the same pointer to the vector, each time.

3) Do not use fread to read the whole thing from a file (and, presumably, write it out to a file in the first place). You cannot do this in C++ when classes are not PODs. You must implement some other different approach of reading and writing your data from files.

P.S.: I also see another fundamental bug in the way the shown code writes the objects to the file in the first place, that's also worth pointing out. A pointer to a base class cannot be converted via reinterpret_cast to a derived class, and expect to get anything meaningful. That's not how reinterpret_cast works, the end result is meaningless, so this would be the 4th issue to fix. Although dynamic_cast would work better, in this case, a more proper approach here, it seems, to solve most of the above problems, is for you to implement discrete virtual functions to read and write, correctly, the contents of each derived class to/from the file.

Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
0

The main problem I identified in the code is that you are storing the same static variable; the last of which is probably corrupt (use of f.eof()). Then you attempt to salvage the situation by calling items.pop_back(). This had no effect because all objects of the derived class have the same potentially corrupt data.

@SamVarshavchik is absolutely right ...you can avoid use of f.eof() by changing the structure of your binary file.

  1. store number of objects in integer variable n
  2. write integer n to file
  3. write n objects to file

When reading from binary file

  1. read integer n from file
  2. for n loops read object from file

I provide a simple illustration here. The code shown below describes a base class

#include <iostream>
#include <fstream>
#include <sstream>
#include <string>
#include <cstring>
#include <list>

using namespace std;

class Person
{
protected:
  char name[30];
  int age;

public:
  Person(void) : name{'\0'}, age(0) { }
  Person(const char *name, const int &age) : name{'\0'}, age(age) { std::strncpy(this->name, name, 29); }
  Person(const Person&) = delete;
  virtual ~Person(void) { }

  virtual int type(void) { return 0; }

  virtual std::string to_string(void)
  {
    std::basic_stringstream<char> sout;
    sout << "[ name: " << name << "; ]";
    return sout.str();
  }
};

Here is a derived class

class Employee : public Person
{
private:
  int id;

public:
  Employee(void) : id(0) { }
  Employee(const char *name, const int &age, const int &id) : Person(name, age), id(id) { }
  Employee(const Employee&) = delete;
  virtual ~Employee(void) { }

  // @override
  int type(void) { return 1; }

  // @override
  std::string to_string(void)
  {
    std::basic_stringstream<char> sout;
    sout << "[ name: " << name << "; age: " << age << "; id: " << id << "; ]";
    return sout.str();
  }
};

Here is a code sample that actually writes/reads objects of derived class to/from a binary file test.txt

int main(int argc, char** argv)
{
  std::fstream file;

  if (1) // write
  {
    file.open("test.txt", std::ios_base::out|std::ios_base::binary);
    if (file.is_open() == 0) { cout << "fail 1..."; return 0; }
    Employee e1("james bond", 19, 10007);
    Employee e2("harry porter", 25, 10011);
    int count = 2;
    file.write(reinterpret_cast<char*>(&count), sizeof(int));
    file.write(reinterpret_cast<char*>(&e1), sizeof(Employee));
    file.write(reinterpret_cast<char*>(&e2), sizeof(Employee));
    file.close();
  }

  std::list<Person*> persons;

  if (1) // read
  {
    file.open("test.txt", std::ios_base::in|std::ios_base::binary);
    if (file.is_open() == 0) { cout << "fail 2..."; return 0; }
    int count = 0;
    file.read(reinterpret_cast<char*>(&count), sizeof(int));
    while ((count--) > 0)
    {
      auto *e = new Employee();
      file.read(reinterpret_cast<char*>(e), sizeof(Employee));
      persons.push_back(e);
    }
    file.close();
  }

  for (auto *ptr : persons) { cout << ptr->to_string() << '\n'; delete ptr; }
  persons.clear();
  cout << "done..\n";
  return 0;
}

And here is the output

[ name: james bond; age: 19; id: 10007; ]
[ name: harry porter; age: 25; id: 10011; ]
done..