How can I print member variable?

Question

#include <iostream>
#include <vector>
#include <string>
using namespace std;

class Item {
protected:
    string modelName;
    int price;
    string itemId; // “modelName-seqNumber”
    static int seqNumber; // initialValue = 1
public:
    virtual void print() const = 0;

    void setname(const string& _name)
    {
        modelName = _name;
    }
    string& getitemId()
    {
        return itemId;
    }
};

class Monitor : public Item {
    const int size;
public:
    int seqNumber = 1;
    Monitor(const string& _name, const int _price, const int _size)
        : size(_size)
    {
        setname(_name);
        price = _price;
        itemId = modelName;
        itemId += '-';
        itemId += seqNumber;
    }
    void print() const {
        cout << modelName << ', ' << itemId << ', ' << price << ',' << size << endl;
    }
};

class CPU : public Item {
    const int speed;
public:
    int seqNumber = 1;
    CPU(const string& _name, const int _price, const int _speed)
        :speed(_speed)
    {
        setname(_name);
        price = _price;
    }
    
    void print() const {
        cout << modelName << ', ' << itemId << ', ' << price << ',' << speed << endl;
    }
};

class ItemList {
    vector<Item*> items;
    void copyItem(const ItemList& list) {
        for (const auto item : list.getItem())
            addItem(item);
    }
public:
    ItemList() = default;
    ItemList(const ItemList& list) {
        copyItem(list);
    }
    ItemList(ItemList& il)
    {
        for (const auto item : il.items)
            items.push_back(item);
    }
    vector<Item*> getItem() const { return items; }

    void addItem(Item* const _item) {
        items.push_back(_item);
    }
    void print() const {
        for (const auto item : items)
            item->print();
    }
    void removeItem(const string& _name)
    {
        for (vector<Item*>::iterator it = items.begin(); it != items.end(); it++)
            if ((*it)->getitemId() == _name)
                it = items.erase(it);
    }
};

Above are classes.

and below are main function.

int main() {
    ItemList list1;
    {
        CPU cpu1("Intel", 200, 5); // modelName, price, speed
        CPU cpu2("Intel", 300, 7);
        list1.addItem(&cpu1); list1.addItem(&cpu2);
        Monitor monitor1("Samsung", 1000, 40); //modelName, price, size
        list1.addItem(&monitor1);
    }
    cout << "Section 1" << endl;
    list1.print();
    {
        ItemList list2(list1);
        cout << "Section 2" << endl;
        list2.print();
        list2.removeItem("Intel-2");
        cout << "Section 3" << endl;
        list2.print();
    }
    {
        ItemList list3;
        list3 = list1;
    }
    cout << "Section 4" << endl;
    list1.print();
}

Hi, I'm studying c++ and I have a question!

I want to print modelName but when I run this code, some strange numbers were printed.

and also ',' wasn't printed.

I tried to debug my code but I failed.

What is the reason? I need your help!

and thank you for your attention!

Looks like you're taking references to a lot of local variables, which then go out of scope. Once a local variable goes out of scope, it's gone forever, and any attempts to access it result in undefined behavior, which is a fancy way of saying "everything is messed up" — Silvio Mayolo, May 21 '21 at 01:45
"I tried to debug my code but I failed". It's definitely good that you tried! Q: What exactly did you try? For example, are you using an IDE (like Eclipse)? Or a command-line debugger (like GDB). Exactly what "didn't work"? — paulsm4, May 21 '21 at 01:46
Please review [ask], in particular the parts about presenting your question before showing any code and reducing your code to a [mre]. Focus on one problem at a time. Maybe -- if you have simplified your code enough -- try adding the reasons you believe your code should work perfectly. — JaMiT, May 21 '21 at 01:54
Related: [Can a local variable's memory be accessed outside its scope?](https://stackoverflow.com/q/6441218/5023438) — alter_igel, May 21 '21 at 02:03

score 1 · Answer 1 · answered May 21 '21 at 01:58

{
    CPU cpu1("Intel", 200, 5); // modelName, price, speed
    CPU cpu2("Intel", 300, 7);
    list1.addItem(&cpu1); list1.addItem(&cpu2);
    Monitor monitor1("Samsung", 1000, 40); //modelName, price, size
    list1.addItem(&monitor1);
}

This declares two objects in the inner scope, cpu1 and cpu2, passes a pointer to them to addItem, which saves them in its internal vector. Same thing for monitor1.

After this scope ends, cpu1, cpu2, and monitor1 go out of scope and get destroyed. Their pointers in the vector are now pointing to destroyed objects, and using those pointers later results in undefined behavior, and the result of that is the garbage output that you are seeing.

I assume the teacher falls into traditional and now dated pattern of C "always pass structures by pointer" i.e. by address, avoiding a copy. One can see that alot in POSIX API. Once upon a time in C that was most effective way, now most calling conventions may imply such. In this case it assumed that class should make copy anyway, but that's C agreement, not C++! — Swift - Friday Pie, May 21 '21 at 03:19

prehistoricpenguin · Answer 2 · 2021-05-21T03:00:57.120

Your code has several bugs, I will point them out one by one.

Pushed the pointed to a local variable that is no longer lived after the first scope

    {
        CPU cpu1("Intel", 200, 5); // modelName, price, speed
        CPU cpu2("Intel", 300, 7);
        list1.addItem(&cpu1); list1.addItem(&cpu2);
        Monitor monitor1("Samsung", 1000, 40); //modelName, price, size
        list1.addItem(&monitor1);
    }

Try to print a string with char type:

cout << modelName << ', ' << itemId << ', ' << price << ',' << size << endl;

Add number to string, which won't work:

itemId += seqNumber;

I have fixed your code by:

Replace the local objects with pointers:

    CPU* cpu1 = new CPU("Intel", 200, 5);  // modelName, price, speed
    CPU* cpu2 = new CPU("Intel", 300, 7);
    list1.addItem(cpu1);
    list1.addItem(cpu2);
    Monitor* monitor1 =
        new Monitor("Samsung", 1000, 40);  // modelName, price, size
    list1.addItem(monitor1);

Add virtual destructor to the base class

  virtual ~Item() = default;

Add clone function for all class, to avoid reference freed objects:

class Monitor : public Item {
  const int size;

 public:
  int seqNumber = 1;
  Monitor(const string& _name, const int _price, const int _size)
      : size(_size) {
    setname(_name);
    price = _price;
    itemId = modelName;
    itemId += '-';
    itemId += std::to_string(seqNumber);
  }
  virtual Item* clone() override {
    return new Monitor(this->modelName, this->price, this->size);
  }
  void print() const {
    cout << modelName << ", " << itemId << ", " << price << ',' << size << endl;
  }
};

Append number to string with std::to_string

itemId += std::to_string(seqNumber);

Add deep copy for the objects:

  void copyItem(const ItemList& list) {
    for (const auto item : list.getItem()) addItem(item->clone());
  }
  ItemList(ItemList& il) {
    for (const auto item : il.items) items.push_back(item->clone());
  }

Add operator=

  ItemList& operator=(const ItemList& list) {
    for (auto* p : items) {
      delete p;
    }
    items.clear();
    copyItem(list);
    return *this;
  }

Call free in the destructor to avoid memory leak

  ~ItemList() {
    for (auto* p : items) {
      delete p;
    }
  }

The remove item function have dealed with iterators wrongly

  void removeItem(const string& _name) {
    for (vector<Item*>::iterator it = items.begin(); it != items.end();)
      if ((*it)->getitemId() == _name) {
        auto pre = *it;
        it = items.erase(it);
        delete pre;
      } else {
          ++it;
      }
  }

Finally the code works smoothly, generally the pointers should be std::unique_ptr or std::shared_pointer here to be mordern c++, if you like to see it, I will update the answer. Online demo

The shared_ptr version is done, easier to maintain, no manually memory management now, I highly recommend it:

#include <iostream>
#include <memory>
#include <string>
#include <vector>
using namespace std;

class Item {
 protected:
  string modelName;
  int price;
  string itemId;         // “modelName-seqNumber”
  static int seqNumber;  // initialValue = 1
 public:
  virtual void print() const = 0;
  virtual ~Item() = default;

  void setname(const string& _name) { modelName = _name; }
  string& getitemId() { return itemId; }
};

class Monitor : public Item {
  const int size;

 public:
  int seqNumber = 1;
  Monitor(const string& _name, const int _price, const int _size)
      : size(_size) {
    setname(_name);
    price = _price;
    itemId = modelName;
    itemId += '-';
    itemId += std::to_string(seqNumber);
  }
  void print() const {
    cout << modelName << ", " << itemId << ", " << price << ',' << size << endl;
  }
};

class CPU : public Item {
  const int speed;

 public:
  int seqNumber = 1;
  CPU(const string& _name, const int _price, const int _speed) : speed(_speed) {
    setname(_name);
    price = _price;
  }

  void print() const {
    cout << modelName << ", " << itemId << ", " << price << "," << speed
         << endl;
  }
};

class ItemList {
  vector<std::shared_ptr<Item>> items;
  void copyItem(const ItemList& list) {
    for (const auto item : list.getItem()) addItem(item);
  }

 public:
  ItemList() = default;
  ItemList(const ItemList& list) { copyItem(list); }
  ItemList(ItemList& il) {
    for (const auto item : il.items) items.push_back(item);
  }
  ItemList& operator=(const ItemList& list) {
    items.clear();
    copyItem(list);
    return *this;
  }
  ~ItemList() = default;
  vector<std::shared_ptr<Item>>& getItem() const { return items; }

  void addItem(std::shared_ptr<Item> _item) { items.push_back(_item); }
  void print() const {
    for (const auto item : items) item->print();
  }
  void removeItem(const string& _name) {
    for (auto it = items.begin(); it != items.end();)
      if ((*it)->getitemId() == _name) {
        it = items.erase(it);
      } else {
        ++it;
      }
  }
};

int main() {
  ItemList list1;
  {
    auto cpu1 =
        std::make_shared<CPU>("Intel", 200, 5);  // modelName, price, speed
    auto cpu2 = std::make_shared<CPU>("Intel", 300, 7);
    list1.addItem(cpu1);
    list1.addItem(cpu2);
    auto monitor1 = std::make_shared<Monitor>("Samsung", 1000,
                                              40);  // modelName, price, size
    list1.addItem(monitor1);
  }
  cout << "Section 1" << endl;
  list1.print();
  {
    ItemList list2(list1);
    cout << "Section 2" << endl;
    list2.print();
    list2.removeItem("Intel-2");
    cout << "Section 3" << endl;
    list2.print();
  }
  {
    ItemList list3;
    list3 = list1;
  }
  cout << "Section 4" << endl;
  list1.print();
}

Online demo

Thanks! but I can't fix main function! Only I can fix my classes. — iamianpark, May 21 '21 at 02:32
@iamianpark Your `main` function is buggy, why won't you fix it? — prehistoricpenguin, May 21 '21 at 02:33
because it's a skeleton that I can't fix. My teacher taught me not to fix it! lol — iamianpark, May 21 '21 at 02:48
@iamianpark Your teacher is wrong this time, all people will make mistakes — prehistoricpenguin, May 21 '21 at 02:57

How can I print member variable?

2 Answers2