Isn't the line QProcess *p redundant?

Question

I've stumbled across this GitHub Project.

While I don't agree with the code in general I can't wrap my mind about the line:

QProcess *p; and p=*it;

Am I wrong when I feel like creating a new pointer there is completely redundant or is there some purpose of it I'm missing for example performance wise?

#define VM_COUNT 202
    cout << "Creating process objects" << endl;
    for(int i=0;i<VM_COUNT; i++)
        vms.push_back(new QProcess(parent));

    cout << "Processes created. Type 'asdf' or someting to start them...";
    string s;
    cin >> s;

    cout << "Starting processes" << endl;

    QProcess* p; // this <------
    QVector<QProcess*>::Iterator it;
    for(it=vms.begin(); it!=vms.end(); ++it){
        p=*it; // this <------
        p->start(command,args); // this <------
    }

I would simply go with:

     for(it=vms.begin(); it!=vms.end(); ++it)
     {
        (*it)->start(command,args); // ty Mike
     }

Yes, it does seem useless, unless they use the last element after the loop. I'd go with `for (auto p : vms) p->start(command, args)` — tux3, Mar 07 '15 at 17:05
If vms is something like a vector then the change seems fine. If xms is a list, then using [] can be much slower. — brian beuning, Mar 07 '15 at 17:08
@brianbeuning last time I checked I think you can't access a QList via `[]` at all. But yes it's a QVector in this case. — deW1, Mar 07 '15 at 17:11
That would be `(*it)->`, not `vms[it]->`, but yes, there's no particular need for the extra variable in this particular loop. Maybe the author likes pointers. — Mike Seymour, Mar 07 '15 at 17:17
Performance-wise, the presence, or absence, of an explicit `p` is likely irrelevant. The compiler has to generate code that fetches the pointer and stores it somewhere. That will be, likely, in a register. So, there will be a `p` whether you care for it or not. It's just a waste of time to write it out explicitly. Perhaps the author didn't know how to write `(*it)->start(...);`... — Kuba hasn't forgotten Monica, Mar 07 '15 at 22:00

score 1 · Accepted Answer · answered Mar 07 '15 at 17:34

It is useful for code portability, or when matching an example. Say you find an example that uses QProcess * p for 100 lines, and then you apply it to a vector of processes, readability suffers when you swap out p for (*iter) in each location... but if you assign a variable with the same name at the top of the local scope, you gain readability/maintainability.

Also Qt supports many types of iterators, STL style, Java style and a boost foreach style. I prefer the foreach style that lends it self to the the least amount of code and the highest readability for me. And it seems to be what the documentation and examples that Qt has tends to prefer, too.

Be sure to read about mutable v immutable, when you are looking at which iterators to use.

QList<QProcess *> listOfProcesses;

// ...

foreach(Process * p, listOfProcesses)
{
    p->start();
}

And on a side note, I might write it once with p, but before I commit or leave the code, I'd do Ctrl+Shift+R in Qt Creator and rename p to process.

Hope that helps.

seems legit however `foreach` as far as I know isn't very performance effective. In general I would simply use a comment to clarify what (*iter) is than to lose performance. — deW1, Mar 07 '15 at 17:45
http://stackoverflow.com/questions/10522155/how-does-q-foreach-foreach-macro-work-and-why-is-it-that-complex http://doc.qt.io/qt-5/containers.html#the-foreach-keyword It looks like it is equivalent to the stl iterator, and its a macro, so it gets swapped before compilation. Losing performance is a moot point. — phyatt, Mar 10 '15 at 14:30

Isn't the line QProcess *p redundant?

1 Answers1