QList and removeAt() question

Hi; I have a little question about QList and memory usage. On my header I have a QList pointer:

Code:

---

QList<MyObjectData> *myList;

---

on class an init method

Code:

---

void MyClass::init(){
 
myList= new QList<MyObjectData>;
 
}

---

periodically I feed my list with new elements :

Code:

---

void MyClass::feedList(int id){
 
MyObjectData mod;
mod.setId(id);
myList->append(mod);
 
}

---

and method called by a timer take this data, use them and removing by calling removeAt(0) :

Code:

---

void MyClass::useData(){
 
if(commandRequest->size()>0 && busy == false)
	{
		busy = true;
		executeSomething((MyObjectData )myList->at(0));
		myList->removeAt(0);
		busy = false;
	}
}

---

can be any memory issue whit these methods? thank you...

You don't show where you delete myList in your code. You should delete in init's counterpart (cleanup?) or your MyClass destructor so that you don't leak memory for the list itself. I would add the following code to do that:

Code:

---

myList.clear(); // done by destructor, but doesn't hurt to clear IMHO
delete myList;
myList = nullptr;

---

Since you are not storing pointers to objects in your list, I don't see any issues with your handling of MyObjectData.

P.S. You will crash if your myList is empty when you run this code, so also check that myList->size() > 0 in your if statement.

Quote:

---

Originally Posted by jefftee

Code:

---

myList.clear();
delete myList; // some people prefer myList->deleteLater() but my pref is to control alloc/dealloc for object instances I manage.
myList = NULL;

---

---

You don't need to clear() explicitly, the destructor does that.
QList does not have a deleteLater() method, it is not a QObject subclass.
The value of a null pointer is either 0 or nullptr :)

Quote:

---

Originally Posted by jefftee

Since you are not storing pointers to objects in your list, I don't see any issues with your handling of MyObjectData.

---

Indeed

Quote:

---

Originally Posted by jefftee

P.S. You will crash if your myList is empty when you run this code, so also check that myList->size() > 0 in your if statement.

---

And there is no need for the cast, QList::at() returns the correct type itself.

Cheers,
_

Quote:

---

Originally Posted by anda_skoa

You don't need to clear() explicitly, the destructor does that.
QList does not have a deleteLater() method, it is not a QObject subclass.
The value of a null pointer is either 0 or nullptr :)

---

Thanks for correcting my post @anda_skoa. For that null pointer assignment, old habits are hard to break! I will update my post so as not to mislead anyone that runs across this in the future.

thank you. In userData method there's already control of array lenght but I forget to rename QList when I post it on forum :

Code:

---

//commandRequest is the real name of my QList, I call it "myList" only in this post
if([B]commandRequest[/B]->size()>0 && busy == false)
	{
		busy = true;
		executeSomething((MyObjectData )myList->at(0));
		myList->removeAt(0);
		busy = false;
	}
}

---

Ok, understood, but you'll get better responses when you provide actual code rather than dummy code for posting to forums. All too often people post code that won't compile or isn't actually the code they're using, or omit something they think isn't relevant, which can obscure the real problem, etc.

BTW, what is your objective in using the busy boolean the way you show in the code?

If your application isn't multi-threaded, then the busy boolean isn't needed because it's not protecting anything and if your code is actually multi-threaded and the QList is accessed by multiple threads concurrently, you should serialize access to the QList by locking/unlocking with a QMutex or use the QMutexLocker convenience class.

Ok thank you but I change only method's name. UseData method is periodically call by a QTimer so I decide to use boolen to prevent multiple launch when executeSomething(MyObjectData) method require more time than QTimer. Useless?

QTimer is event based.
As long as your slot is executed, no further events can be processed.
The timer can only fire again after execution has returned to the event loop.

Cheers,
_

ah ok, thank you :)