3

This error occurs during run time, and I'm not sure what's causing it - the code looks correct to me.

#include <iostream>
#include <string>

using namespace std;

struct Room {
    int d_noSeat;
    bool d_hasProjector;
    Room() = default;
    Room(const Room& r);
};

class Event {
    Room* d_room;
    std::string d_name;
public:
    Event();
    Event(const Event& e);
    ~Event();
    void set(Room r, const std::string& name);
    void print();
};

Event::Event() : d_room(0), d_name("") {};

void Event::print() {
    std::cout << "Event: " << d_name;
    if (d_room != 0) {
        std::cout << " in size " << d_room->d_noSeat;
        if (d_room->d_hasProjector)
            std::cout << " with";
        else
            std::cout << " without";
        std::cout << " projector";
    }
    std::cout << std::endl;
    return;
}

void printEvent(Event e) {
    e.print();
    return;
}


void Event::set(Room r, const std::string& name) {
    d_room = &r;
    d_name = name;
}

// Room shallow copy constructor
Room::Room(const Room& r) : 
    d_noSeat(r.d_noSeat), 
    d_hasProjector(r.d_hasProjector)
{ }

// Event deep copy constructor
Event::Event(const Event& e) : 
    d_name(e.d_name), 
    d_room(new Room(*e.d_room))
{ }

// Event destructor
Event::~Event()
{
    delete[] d_room;
}


int main() {
    const int noLect = 5;
    Room r;
    Event lectures[noLect];

    for (int i = 0; i < noLect; ++i) {
        r.d_noSeat = i + 1;
        r.d_hasProjector != r.d_hasProjector;
        lectures[i].set(r, "CSI2372");
        lectures[i].print();
    }
    std::cout << "-------------------" << std::endl;
    for (int i = 0; i < noLect; ++i) {
        printEvent(lectures[i]);
    }
    return 0;
}

The error apparently occurs at line 52 (first line in the print() function). In addition to this, the printed text displays numbers that are very large and often negative. What is causing this?

Will
  • 59
  • 1
  • 1
  • 6
  • Does this answer your question? [Why do I get \_CrtIsValidHeapPointer(block) and/or is\_block\_type\_valid(header->\_block\_use) assertions?](https://stackoverflow.com/questions/64418624/why-do-i-get-crtisvalidheappointerblock-and-or-is-block-type-validheader-b) – ead Nov 02 '20 at 14:26

2 Answers2

9

Issue

void Event::set(Room r, const std::string& name) 
{
    d_room = &r;
    //      ^
    d_name = name;
}

You are referencing to the temporary object: Room r passed by value, which is destroyed at the end of the scope: }.

Instead you must reallocate the member pointer:

d_room = new Room(r);

Why it went wrong

Because you are writing C-style code in C++ classes.

In C++ we tend to:

  1. Avoid naked pointers, prefer smart pointers:

    class Event 
    {
        std::shared_ptr<Room> d_room;
     ...
    
    Event::~Event() { /* no need to delete */ }
    
  2. Use constructor overloading (instead of using set-like functions after construction):

    Event(Room& r, const std::string& name):
            d_room(new Room(r)),
            d_name(name)
    {}
    
  3. Pass by reference:

    void set(Room& r, const std::string& name);
    
  4. Avoid raw arrays, use STL facilities instead:

    std::vector<Event> lectures;
    // or
    std::array<Event, 5> lectures;
    

Another issue

r.d_hasProjector != r.d_hasProjector; // checks if r.d_hasProject is not itself

You probably want

r.d_hasProjector = !r.d_hasProjector;

Complete code: link

Also, here is a must-read link about advanced C++ stuff which, I believe, will be very useful to you: http://www.parashift.com/c++-faq/

Edit: I forgot about your question:

In addition to this, the printed text displays numbers that are very large and often negative. What is causing this?

Those numbers are garbage. Variables that are not explicitly initialized are not initialized at all. Memory is allocated but holds old information from previous program. It could contain anything. When you read from uninitialized variables, you'll get this garbage. You had a pointer which was pointing to a destroyed object. So the pointer was effectively uninitialized.

ixjf
  • 29
  • 2
  • 7
Ivan Aksamentov - Drop
  • 12,860
  • 3
  • 34
  • 61
  • This is perfect, thank you very much. Answered all of my questions, and gave some nice tips. – Will Oct 30 '13 at 18:13
  • Why are you using `Room& r` as opposed to `const Room& r`, it's a pure input is it not? We don't expect nor want it to change inside the function, same as the name string that actually was given as const reference. – DaedalusAlpha Apr 30 '14 at 13:36
  • @DaedalusAlpha you're right, it must be `const` ref. I overlooked that somehow. However, I think it's better to pass a string by value here, so C++11 move semantics could work. – Ivan Aksamentov - Drop Apr 30 '14 at 15:02
1

Your problem is here:

void Event::set(Room r, const std::string& name) {
    d_room = &r;
    d_name = name;
}

The &r takes the address of an object whose lifetime ends when the function returns, resulting in undefined behaviour when you later try to access it.

If you want to use pointers, you need to allocate them dynamically:

void Event::set(Room* r, const std::string& name) {
    d_room = r;
    d_name = name;
}

// ...
for (int i = 0; i < noLect; ++i) {
    Room* r = new Room;
    r->d_noSeat = i + 1;
    r->d_hasProjector != r.d_hasProjector;
    lectures[i].set(r, "CSI2372");
    lectures[i].print();
}
// ...

But it doesn't look like you need pointers here, you should be able to have

Room d_room;

in the Event class.

molbdnilo
  • 64,751
  • 3
  • 43
  • 82
  • So what's happening is the address being passed to d_room is being destructed at the end of the Event::set(...) function? Would there be a way to implement this function, keeping the use of pointers, but without altering the main method? That was given and the rest of the code is based off of it. Thanks for your help though, you've cleared things up. – Will Oct 30 '13 at 17:54
  • No need to pass by pointer, reference will be enough. No need to allocate `Room* r = new Room;` outside of the class. No one knows who and when to `delete` it. Finally it will leak. – Ivan Aksamentov - Drop Oct 30 '13 at 18:19
  • @Drop Using a reference to a parameter is just as bad as using a pointer. Where you allocate the Room depends on who owns it. – molbdnilo Oct 30 '13 at 18:41
  • @user2933906 If `main` was given to you, and you're required to use pointers, it's faulty and you should tell whoever gave it to you to take a basic course in C++. – molbdnilo Oct 30 '13 at 18:44