Copying vector of pointers to another vector

[Momergil]

Hello!

I'm having a problem in trying to copy the content of a vector to another vector.

Essentially what I want is to create a vector of pointer


Alarm *vector[];

and add them pointers that point to objects


for (...;...;...)
{
vector[] = new Alarm;
etc.
}

than I want to pass that vector to a different class through a function that will makes a different vector


Alarm *alarms[];

that will "copy" the content of the first, i.e. will point to the same objects of the first one in the same sequence. I though that I should do the following:


for (...;...;...)
{
alarms[] = vector[];
etc.
}

but despite the compiler runs OK, when I try to use the software it crashes, displaying the message that the software stop working and I must close.

Notice, though, that this only happens when the copy is done in this second class. If the same copy is done in the original class, everything runs fine! So, what is the problem?

Details:

I'm calling the following method to do the copy:



v_alarmmanagment->readExistingAlarms(v_vector, numberofalarms);

/***************/

void AlarmManagment::readExistingAlarms(Alarm *alarms[], int numb)
{
vector2[numb] = alarms[numb];
}



Thanks!


Momergil

-----
Edit: only now I saw that I should have posted this thread in the "General Programming" section. Sorry!

[Zlatomir]

This code:

void AlarmManagment::readExistingAlarms(Alarm *alarms[], int numb)
{
vector2[numb] = alarms[numb];
}
will not do what you want, this will copy the last value (pointer), leaving the rest uninitialized and most likely you get "seg-fault" or "access violation" errors.

See this example with c-arrays (i use int data as example) and i will later give you one that uses c++ (and Qt's nice classes and depending on what you do with the "copy" of the pointers it might no even copy them)
So, the C with classes code:


void copy_and_modify_array(int* v[], int size) {
/*
because this declaration:
int* v_copy[size] ...
will fail on most compilers
...
this is how you should declare it...
*/
int** v_copy = new int*[size];

//loop and copy the pointers
for(int i = 0; i != size; ++i) {
v_copy[i] = v[i];
}

//modifying the "vector copy" pointed values will affect the original
for(int i = 0; i != size; ++i) {
*(v_copy[i]) = *(v_copy[i])+1000; //add 1000 to the old value
}
}

int main()
{
const int size = 3;
int* vector[size];
//init vector
for(int i = 0; i != size; ++i) {
vector[i] = new int(i);
}
//print before
for(int i = 0; i != size; ++i) {
std::cout << *vector[i] << " ";
}
std::cout << '\n';
//call modify
copy_and_modify_array(vector, size);

//print again after modifications
for(int i = 0; i != size; ++i) {
std::cout << *vector[i] << " ";
}
return 0;
}

And now the real solution to your problem will be to use QVector (http://doc.qt.nokia.com/latest/qvector.html) or QList (http://doc.qt.nokia.com/latest/qlist.html) and QSharedPointer (http://doc.qt.nokia.com/4.7-snapshot/qsharedpointer.html) - this solution might not copy the pointers*** because the QVector is sharing data.
***depends on what you do with the copy.
Here is the code:

#include <iostream>
#include <QVector>
#include <QSharedPointer>

using namespace std;

void copy_and_modifie_QVector(QVector<QSharedPointer<int> > v) //you don't need to pass the size (QVector knows it's size)
{
QVector<QSharedPointer<int> > vector_copy = v;

//the QShaderPointer "copy" will modifie the original data (because that data is shared with the original copy of the pointers)
foreach(QSharedPointer<int> sharedPointer, vector_copy) //easy way to go thru all QVector elements //you can still use for loop, but this looks nice ;)
{
*sharedPointer = *sharedPointer + 1000;
}
}

int main()
{

QVector<QSharedPointer<int> > vector;
vector.resize(3);
//init vector
for(int i = 0; i != 3; ++i)
{
vector[i] = QSharedPointer<int>(new int(i));
}

//print before
for(int i = 0; i != 3; ++i)
{
std::cout << *vector[i] << " ";
}

std::cout << '\n';
//call function that modifie the vector
copy_and_modifie_QVector(vector);

//print again after modifications
for(int i = 0; i != 3; ++i)
{
std::cout << *vector[i] << " ";
}

std::cin.get();
return 0;
}

[Momergil]

Hello, Zlatomir!

First, thanks for the help. just some comments:


This code:

void AlarmManagment::readExistingAlarms(Alarm *alarms[], int numb)
{
vector2[numb] = alarms[numb];
}
will not do what you want, this will copy the last value (pointer), leaving the rest uninitialized and most likely you get "seg-fault" or "access violation" errors.

Well, that was actually a simplified version of the code. Actually I use for(;;) to copy all the vectors, but the problem appears even if a copy just one of them as in the code I posted.

Anyway, thanks for the codes. I will see the second one soon.


God bless,

Momergil.

[ChrisW67]

If there are "numb" elements in the array you have space allocated for "numb" values then the last valid index is "numb-1". Trying to access array[numb] is a recipe for out-of-bounds memory violations.
Where is space to hold the array of pointers, called vector2, allocated? I suspect you are never doing that.

Don't try to fix this using C-style memory management techniques... save yourself the anguish and use either the STL containers or the Qt containers as Zlatomir suggests.

[Zlatomir]

And another "little" difference between my two examples is that the first example leaks memory (i intentionally "forgot" about cleanup and while writing the post i forgot to mention this advantage) and the second example automatically delete the allocated memory so there is no leak in the second example.

[Momergil]

If there are "numb" elements in the array you have space allocated for "numb" values then the last valid index is "numb-1". Trying to access array[numb] is a recipe for out-of-bounds memory violations.

Hello Chris,

well this is not actually in my test code, i think I copied wrong. Here is the code that is not working:


void AlarmManagment::readExistingAlarms(Alarm *alarms[], int numb)
{
/**** Copy current data ****/
numberofalarms2 = numb;

for (int aaa = 0; aaa < numberofalarms2; aaa++)
{
v_Alarm2[aaa] = alarms[aaa];
qDebug() << "Copied: " << v_Alarm2[aaa]->getDia();
qDebug() << "From: " << alarms[aaa]->getDia();
}

/**** Add to the list all alarms ****/
for (int zzz = 0; zzz < numberofalarms2; zzz++)
{
if (!vector_String1.contains(v_Alarm2[zzz]->getDia()))
{
vector_String1.push_back(v_Alarm2[zzz]->getDia());
qDebug() << "Antes do addRoot 2";
addRoot(v_Alarm2[zzz]->getDia(), v_Alarm2[zzz]->getDiadaSemana(), v_Alarm2[zzz]->getHora(),
v_Alarm2[zzz]->getSmallDescription());

addToYearandMonthCB(v_Alarm2[zzz]->getDiaOnlyYear(),v_Alarm2[zzz]->getDiaOnlyMonth());
}
else
{
listaDeQTreeWidgetItems = ui->Listofalarms->findItems(v_Alarm2[zzz]->getDia(),Qt::MatchExactly,0);
addChild(listaDeQTreeWidgetItems.front(), v_Alarm2[zzz]->getDiadaSemana() ,v_Alarm2[zzz]->getHora(),
v_Alarm2[zzz]->getSmallDescription());
}
}
}


When I run the Debugger, it points error not in the process of copy (which my test show that runs fine), but in the addRoot() function. The addRoot() function is:


void AlarmManagment::addRoot(QString data, QString semana, QString hora, QString smadesc)
{
QTreeWidgetItem *itm = new QTreeWidgetItem(ui->Listofalarms);
itm->setText(0,data);

QTreeWidgetItem *itm2 = new QTreeWidgetItem();
itm2->setText(0,semana);
itm2->setText(1,hora);
itm2->setText(2,smadesc);

itm->addChild(itm2);
}


and the error is pointed to the first line of this function. Very interestingly, if I commet all the second for(;;) where the addRoot() function is, no problems occurs, so it is like the way I copy the pointers is not problematic, but makes another part of the code to be so.


Where is space to hold the array of pointers, called vector2, allocated? I suspect you are never doing that.

Hmm what exactly do you mean by that? :)


Thanks,

Momergil

[ChrisW67]

At line 20 of the first listing you are calling addRoot(). I would suspect the pointer, v_Alarm2[zzz], is either 0, uninitialised, or pointing at a deleted item.

The array v_Alarm2 that you are copying into has to have had space allocated for it somewhere. It might be declared with a fixed size, or dynamically allocated, but we cannot see that.

Have you considered QVector<> or QList<> in place of the bare C++ array?

[Momergil]

At line 20 of the first listing you are calling addRoot(). I would suspect the pointer, v_Alarm2[zzz], is either 0, uninitialised, or pointing at a deleted item.

The array v_Alarm2 that you are copying into has to have had space allocated for it somewhere. It might be declared with a fixed size, or dynamically allocated, but we cannot see that.

Have you considered QVector<> or QList<> in place of the bare C++ array?

Hello Chris,

Now I think it's strange that v_Alarm2[zzz] would be "either 0, uninitialised, or pointing at a deleted item." because the test done in lines 9 and 10 of the first code showed that the copy was done correctly; the v_Alarm2 is trully pointing to the objects Alarms received by Alarm *alarms[]:



Starting D:\- Meus Documentos\Minhas obras\Softwares\mClock\mClock 1.05\debug\mClock.exe...
"D:/- Meus Documentos/Minhas obras/Softwares/mClock/mClock 1.05"
Alarme invalido; ja passou da data: QDate("sex set 23 2011")
Copied: "29/10/2011"
From: "29/10/2011"
Copied: "29/11/2012"
From: "29/11/2012"
Copied: "29/08/2013"
From: "29/08/2013"
Copied: "30/08/2013"
From: "30/08/2013"
Copied: "30/08/2013"
From: "30/08/2013"
Antes do addRoot 2
The program has unexpectedly finished.
D:\- Meus Documentos\Minhas obras\Softwares\mClock\mClock 1.05\debug\mClock.exe exited with code -1073741819


More than that, the debugger don't point the problem to line 20, what would indicate a problem in the calling addRoot() with the vector, but to line 3 of the second box, a line that has nothing to do with v_Alarm2[], rather being a simple declaration of a new QTreeWidgetItem pointer.

About the space allocated for v_Alarm2, well I didn't declare it with a fixed size, rather as:


Alarm *v_Alarm2[];

in the header file. And in terms of using QVector<> or else QList<>, in fact I pretended to use QVector<> in the beginning, but a part of my code used some unusual coding that I already used 2 years ago in my classes about C++ in the university and I decided to use exactly the same code for this software for the time being. In any case, how would I declare "Alarm *v_Alarm2[]" using QVector<>?


Thanks!


Momergil

[wysota]

"Alarm *v_Alarm2[]" is equivalent to "Alarm **v_Alarm2". It doesn't allocate any space for the data. If you start writing data to such array you risk overwriting critical areas of the memory (the stack frame) which can cause a crash when data that used to occupy the overwritten area of memory is tried to be accessed. I don't know if this is the case here but it might be.

As for using C-style arrays vs C++ vectors... "Alarm *v_Alarm2[]" is a C construction, not a C++ one. However all things you can do with a C array, you can do with a vector as well (apart from overwriting data that shouldn't be overwritten).

[Momergil]

"Alarm *v_Alarm2[]" is equivalent to "Alarm **v_Alarm2". It doesn't allocate any space for the data. If you start writing data to such array you risk overwriting critical areas of the memory (the stack frame) which can cause a crash when data that used to occupy the overwritten area of memory is tried to be accessed. I don't know if this is the case here but it might be.

As for using C-style arrays vs C++ vectors... "Alarm *v_Alarm2[]" is a C construction, not a C++ one. However all things you can do with a C array, you can do with a vector as well (apart from overwriting data that shouldn't be overwritten).

Hello wysota,

in fact the compiler, some times (depending on how I arranged things), did called the v_Alarm2[] as being **v_Alarm2 as you noticed. But in any case, how should I, than, create a vector of pointers and than create pointers to an object and store those pointers in the vector? That is what I pretend to do.

Thanks,


Momergil

[wysota]

But in any case, how should I, than, create a vector of pointers and than create pointers to an object and store those pointers in the vector?

The bare C++ way:


Alarm **v_Alarm2 = new Alarm*[10];
for(int i=0;i<10;++i)
v_Alarm2[i] = new Alarm[10];
The STD/Qt C++ way:

QVector< QVector<Alarm> > v_Alarm2;
v_Alarm2.resize(10);

The best approach: since there are no two dimensional arrays in C++, it is better to create a one dimensional array and provide a way to access the data using two-dimensional coordinates.

QVector<Alarm> v_Alarm2(100);

Alarm& getAlarm(QVector<Alarm> &arr, int x, int y) {
return arr[y*10+x];
}

[Momergil]

Hello wysota,

It seems to me that nether of the possible solutions you presented work. In all cases ether the compiler return 25 errors or else made the software chrases. When I try the first code, it gives a error in the QString that I simply don't comprehend. If I try the second code, I'm unable to do what I want: the first vector only allows me to acces functions of the second vector, not the functions of the object I want to work with. And the third I didn't understand exactly how should I use it :P So, if that will make it clearer (maybe the fault is mine, that means maybe I'm not describing exaclty what I want), here is the code I have now and that works: (part of it)


//Header
Alarm *v_Alarm[];



//.cpp
while (...)
{
//Create a new alarm with its atributes
v_Alarm[numberofalarms] = new Alarm;
v_Alarm[numberofalarms]->setDia(temp_date);
v_Alarm[numberofalarms]->setDiadaSemana(temp_diasemana);
v_Alarm[numberofalarms]->setHora(temp_time);
v_Alarm[numberofalarms]->setDescription(description_str); //Talvez nao seja isso.
v_Alarm[numberofalarms]->setSmallDescritpion(smalldescription_str);
v_Alarm[numberofalarms]->setName(nome_str);

if (!soundfile.isEmpty())
v_Alarm[numberofalarms]->setSound(soundfile);
else
v_Alarm[numberofalarms]->setSound("normal");

numberofalarms++;
}


A new question that appeared now is: couldn't I do a similar thing, but without pointers (rather with the objects themselfs) in the following way:



// Header
QVector<Alarm> vec_Alarm;

//CPP
while (...)
{
Alarm aha;
vec_Alarm.push_back(aha);
vec_Alarm[numberofalarms].setDia(temp_date);
vec_Alarm[numberofalarms].setDiadaSemana(temp_diasemana);
//etc

numberofalarms++;
}

?? Because if I can, that it seems to me that this is fine (the compiler pointed no errors). The unique question that appears is how should I solve my initial problem using this new code.


Thanks,


Momergil

Added after 22 minutes:

Ok, forgot the last line in the last thread: the compiler does return problem. It says:

c:\QtSDK\Desktop\Qt\4.7.3\mingw\include\QtCore\qob ject.h:309: error: 'QObject::QObject(const QObject&)' is private
D:\- Meus Documentos\Minhas obras\Softwares\mClock\mClock 1.06\alarm.h:9: error: within this context
c:\QtSDK\Desktop\Qt\4.7.3\mingw\include\QtCore\qob ject.h:309: error: 'QObject& QObject::operator=(const QObject&)' is private
D:\- Meus Documentos\Minhas obras\Softwares\mClock\mClock 1.06\alarm.h:9: error: within this context

Does somebody knows what is the problem exactly? The "within this context" points to the header of the class Alarms, which is essentially:



#ifndef ALARM_H
#define ALARM_H

#include <QObject>
#include <QDate>
#include <QTime>

class Alarm : public QObject
{
Q_OBJECT
public:
explicit Alarm(QObject *parent = 0);
void setHora(QTime a);
void setDia(QDate b);
...

signals:

public slots:

private:
QDate data, diadasemana;
QTime hora;
...
};

#endif // ALARM_H



Thanks,

Momergil.

[wysota]

It seems to me that nether of the possible solutions you presented work.
Tough luck, they all work fine for me.

When I try the first code, it gives a error in the QString that I simply don't comprehend.
I don't use QString anywhere so obviously the errors you get are unrelated to my code.


If I try the second code, I'm unable to do what I want: the first vector only allows me to acces functions of the second vector, not the functions of the object I want to work with.
I have no idea what you mean by that. Since the outside array is an array of arrays, it seems obvious that operating on the outside array will cause access to the inside array and not to the contents of the inside array.


And the third I didn't understand exactly how should I use it
What exactly you don't understand?