Here's your original code:
for(int i=1; i<=event->mimeData()->urls().length(); i++){
QUrl url
= event
->mimeData
()->urls
().
takeAt(i
-1);
QString file1
= file2.
mid(7,file2.
length()-2);
//removing some things I don't want QString extension
= file1.
right(3);
//taking last 3 letters from the string.. if(extension=="png" || extension=="gif"){ //add to the listwidget
item->setText(file1);
ui->listWidget->addItem(item);
}
}
for(int i=1; i<=event->mimeData()->urls().length(); i++){
item=new QListWidgetItem;
QUrl url = event->mimeData()->urls().takeAt(i-1);
QString file2 = url.toString();
QString file1 = file2.mid(7,file2.length()-2); //removing some things I don't want
QString extension = file1.right(3); //taking last 3 letters from the string..
if(extension=="png" || extension=="gif"){ //add to the listwidget
item->setText(file1);
ui->listWidget->addItem(item);
}
}
To copy to clipboard, switch view to plain text mode
I don't understand why the code provided is better than my first posted, so i really want to understand my mistake in order to avoid it to the feature?
And so you're saying you still don't understand why it runs so slowly?
OK, others have pointed this out, but maybe it wasn't clear. Aside from all the strange stuff you are doing to the string once you get it out of the URL, the main reason why your original code runs slowly is this:
event->mimeData()->urls()
event->mimeData()->urls()
To copy to clipboard, switch view to plain text mode
You're calling this string of methods *twice* for every time through the loop, once when counting the length of the urls in the for statement, and again when you retrieve the QUrl.
You're making three method calls in that single line: retrieving the mimeData from the event, then retrieving the urls from the mimeData, then doing something with the urls (like getting the length or a specific URL).
One (or more) of those three calls is very time intensive. Maybe it's getting the mimeData from the event, maybe it's parsing the mimeData to retrieve the URLs. Doesn't matter. It's the cause of the slowness. String manipulations and comparisons are blindingly fast compared to that.
What does matter is that information is what is called "loop invariant". It is essentially a constant for the entire duration of the loop. So, move it outside the loop as Sven first suggested. Then, you retrieve the mimeData from the event exactly once, and you parse the mimeData into the urls exactly once, and all the rest of the loop simply deals with the urls collection. If I were writing it, I'd even retrieve the urls length into a local variable (int nUrls = urls.length()) and use that as the loop termination condition instead of calling length() repeatedly. I'd also use a more descriptive name for the loop iterator (instead of a generic "i") so that next time I read the code, it is clear to me that I am iterating over urls and not something else.
const QList<QUrl> & urlList = event->mimeData()->urls();
int nUrls = urlList.length();
for ( int nUrl = 0; nUrl < nUrls; nUrl++)
{
// ...
}
const QList<QUrl> & urlList = event->mimeData()->urls();
int nUrls = urlList.length();
for ( int nUrl = 0; nUrl < nUrls; nUrl++)
{
// ...
}
To copy to clipboard, switch view to plain text mode
Hope this helps. No flames please from those who would complain that I'm "doing someone's homework". I think the OP really doesn't understand why his original code is slow despite all the other help.
Bookmarks