C++ question

mzimmers

I realize that this is (mostly) not a Qt question, but...this place has never let me down, so...

struct Bottle : public QObject {
public QObject {
  Q_OBJECT
  ...
}
typedef QList<Bottle *> Bottles;
Bottles m_bottleList;
...
for (it = m_bottleList.begin(); it != m_bottleList.end(); ++it) {
  (*it)->m_amountNeeded = 0;
}

So, I have a Bottle struct, and I create a QList of pointers to instances of these structs (using the append() function of QList). Everything seems to work, but...I have to dereference my iterator (and therefore put it in parens). This not only looks weird, but I can't break it down in the debugger.

Can one of you C++ mavens give me a better idea on how to do this?

Thanks...

Chris Kawa

A simpler way:

for (auto bottle : qAsConst(m_bottleList))
{
    bottle->m_amountNeeded = 0;
}

The qAsConst is to make sure you don't detach unnecessarily.

As for iterators - unless you want to actually modify contents of the list you should use const variants of the iterators, so cbegin() and cend(). Btw. typedefs are pretty old school these days too. A more modern replacement is using statement:

using Bottles = QList<Bottle *>;

mzimmers

@Chris-Kawa "old school" -- well, that's definitely me. But I'm always happy to try new things, and these appear to work. Thanks, Chris.

VRonin

I'm a forever lover of stl-style iterators:

for (auto it = m_bottleList.begin(), itEnd = m_bottleList.end(); it != itEnd ; ++it) {
Bottle *const currentBottle = *it;
currentBottle->m_amountNeeded = 0;
}

mzimmers

@VRonin it's definitely easier to understand, isn't it. All these newer (C11+++) constructs are still somewhat foreign to me (I'm still getting used to the new use for "auto" [*]), but that code is at least clear. I'd done the same trick with creating a local variable so I could see the contents of the element in the debugger.

[*] and yes, all the best rock music was written before 1980. Deal with it, kids...

Chris Kawa

If you don't want to use auto that's fine:

for (Bottle* bottle : qAsConst(m_bottleList))
{
    bottle->m_amountNeeded = 0;
}

just a couple letters more, no biggie.

As for the iterators - for me, now that we have range for, it's just way too much typing/reading. But that's just me, to each their own.
Anyway, just please at least mind the cbegin()/cend() thing. It's not so much important on the standard containers, but very on the Qt ones, because of implicit sharing and possible detach penalty.

mzimmers

@Chris-Kawa noted about cbegin()/cend(). I have several loops like the one I posted. The list of pointers doesn't change, the structs they point to do. Eg:

      for (auto bottle : qAsConst(m_bottleList)) {
        bottle->m_amountNeeded = 0;
      }

If I were to use iterators, I could use cbegin()/cend() here, right?

Chris Kawa

@mzimmers said:

If I were to use iterators, I could use cbegin()/cend() here, right?

Yes, should even, as the iterators point to the pointers so they can be const. You can still modify the objects through const pointers.
You'd only use non-const begin/end if you wanted to change the pointers themselves.

As a side note - containers do have a const overload for normal begin/end so you could stick to that, but you'd have to make sure the container itself is const for that to be selected, so:

const Bottles& make_sure_it_is_const = m_bottleList;

for (auto it = make_sure_it_is_const.begin(), itEnd = make_sure_it_is_const.end(); it != itEnd ; ++it) {
Bottle *const currentBottle = *it;
currentBottle->m_amountNeeded = 0;
}

But that's excessively verbose, at least for me. It's way better to get in habit of using cbegin/cend if you want to stick to iterators.

mzimmers

@Chris-Kawa got it (I think).

I was wrong about the list not changing; I sort it later in the routine. Got some weird compiler messages (that weren't much help in understanding the problem) but got it sorted out.

Thanks again.

kshegunov

@Chris-Kawa said in C++ question:

But that's excessively verbose, at least for me.

Also there's the Qt-style naming: constBegin(), constEnd(), which is my personal preference. And while I support the idea of using a range-based-for, that's useful whenever the iterator is simple (e.g. a glorified C pointer). If you're iterating a map, you either are stuck to explicitly iterating over or going to a structured binding syntax (which I personally despise).