-1

I have a class that contains an array of object pointers as its member variable. I'm currently having an issue in getting the compiler to copy an object to the end of the array as when I step through the program the array of objects reads that its memory cannot be read. Anyone know what the issue might be?

void Notifications::operator+=(const iMessage& src) {
    iMessage** temp2 = nullptr;
    temp2 = new iMessage*[size+1];
    if (size != 0){
        for (int i = 0; i < size; i++) {
            *temp2[i] = *messages[i];
        }
    }
    *temp2[size] = src; //compiler states that it cannot read the data from temp2 after this point
    delete[]messages;
    for (int i = 0; i < size + 1; i++) {
        *messages[i] = *temp2[i]; //Unhandled exception at 0x00C58F99 in w5.exe: 0xC0000005: Access violation reading location 0x00000000.
    }
    size++;
}

Notifications.h

#include "iMessage.h"
#include <vector>

namespace w5 {
    class Notifications {
        int size;
        iMessage **messages;
    public:
        Notifications();
        Notifications(const Notifications&);
        Notifications& operator=(const Notifications&);
        Notifications(Notifications&&);
        Notifications&& operator=(Notifications&&);
        ~Notifications();
        void operator+=(const iMessage&);
        void display(std::ostream&) const;
    };
}

IMessage.h

#ifndef _I_MESSAGE_H_
#define _I_MESSAGE_H_

// Workshop 5 - Containers
// iMessage.h

#include <iostream>
#include <fstream>

namespace w5 {
    class iMessage {
    public:
        virtual void display(std::ostream&) const = 0;
        virtual iMessage* clone() const = 0;
        virtual bool empty() const = 0;
    };

    iMessage* getMessage(std::ifstream&, char);
}
#endif

Message.h

#include "iMessage.h"
namespace w5{
    class Twitter : public iMessage {
        std::string msg;
    public:
        Twitter(char, std::ifstream&);
        virtual void display(std::ostream&) const;
        virtual iMessage* clone() const;
        virtual bool empty() const;
    };

    class Email : public iMessage {
        std::string msg;
    public:
        Email(char, std::ifstream&);
        virtual void display(std::ostream&) const;
        virtual iMessage* clone() const;
        virtual bool empty() const;
    };
}
karsius
  • 63
  • 6
  • 2
    Just use a `vector>`. And your `iMessage` should have a virtual destructor. Beyond that, the compiler says it cannot read the array and then crashes? As in, it produces an internal compiler error? I find that hard to believe. What compiler, version, and what's the error message you get? – Sebastian Redl Dec 04 '14 at 13:24
  • I can recommend to re-organize the function. Cannot accept the "way of thinking", sorry. – i486 Dec 04 '14 at 13:25
  • You're indirecting through `temp2[i]` before you allocate space for it. – Barmar Dec 04 '14 at 13:25
  • Maybe that should be `temp2[i] = messages[i];` or `temp2[i] = new iMessage; *temp2[i] = *messages[i];` – Barmar Dec 04 '14 at 13:27
  • Sorry, excuse the hyperbole. It throws an exception as it cant read the memory. I also cannot use a vector for this. It needs to be a pointer to a pointer – karsius Dec 04 '14 at 13:28
  • I am confused: Is this a compile time error or a run time error. Secondly why must you have a pointer to a pointer. – Martin York Dec 04 '14 at 13:30

3 Answers3

0

First you do

delete[]messages;

then you do

*messages[i] = *temp2[i];

attempting to access the array you've just deleted. I think you just want to take the pointer to the array you've just created:

messages = temp2;

You also do

*temp2[size] = src;

when temp2[size] doesn't point to anything. That should probably be

temp2[size] = src.clone();

to make a persistent copy of the argument and store it in the array.

It's rather tricky to follow this weird pointer-juggling; I think you also want to delete each element of messages before messages itself to avoid leaks. Why not just use std::vector to take care of memory allocation for you? That will reduce the whole insane dance to

std::vector<std::unique_ptr<iMessage>> messages;

void operator+=(const iMessage & src) {
    messages.emplace_back(src.clone());
}

Also, _I_MESSAGE_H_ is a reserved name. You should remove the leading underscore.

Community
  • 1
  • 1
Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
0

1) Just use vector.

2) You should always post exact compiler messages. "compiler states that it cannot read the data from temp2 after this point" is not good enough.

3) You allocate an array of pointers, and then dereference those pointers, but you never let the pointers point anywhere.

4) You delete the messages array and then proceed to copy back into it as if it was still there. (What you actually want to do is just assign messages = temp2.)

5) You're slicing objects all over the place, by using assignment to attempt to copy iMessage objects. There's a reason iMessage has a clone() function.

Sebastian Redl
  • 69,373
  • 8
  • 123
  • 157
0

You want to convert a const reference into a non-const pointer. I wonder that the compiler doesn't throw errors. Which compiler you use?

Is something like this not possible?

void Notifications::operator+=(iMessage* src) {

I was not testing but this should also work:

void Notifications::operator+=(iMessage& src) {
    *bar[foo] = &src;
dgrat
  • 2,214
  • 4
  • 24
  • 46