Should QAbstractItemModel::index(row, column, parent) check for invalid inputs?

Question

Subclassing QAbstractItemModel, I have implemented my own index() method as required. I currently check for valid inputs, but I'm wondering if that's correct. I'm wondering if it's ever valid to create an index for a non-existent piece of data? Perhaps while inserting a row or column?

Code:

QModelIndex LicenseDataModel::index(int row, int column, const QModelIndex & /*parent*/) const
{
    /// TODO: Is this necessary? Should we avoid creating invalid indexes? Or could this
    /// be a bug?
    if (validRowColumn(row, column))
        return createIndex(row, column);
    return QModelIndex();
}

Answer 1

[ I have a better answer. :-) ]

Although I merely repeat what Giuseppe D'Angelo from KDAB has to say about this...

You must distinguish between an invalid and an ill-formed QModelIndex:

About invalid indices (from Navigation and model index creation in Qt's Model/View Programming guide):

QAbstractItemModel::parent(): Provides a model index corresponding to the parent of any given child item. If the model index specified corresponds to a top-level item in the model, or if there is no valid parent item in the model, the function must return an invalid model index, created with the empty QModelIndex() constructor.

This explains the meaning of index.isValid():
A valid index refers to an existing item, the invalid index refers to the root of all items.

Giuseppe D'Angelo first notes that an invalid index (with .isValid() returning false) is still a valid input to functions like rowCount(), hasChildren() etc.

But a QModelIndex can also be ill-formed. It can have a non-existing row or column index, and it can even be from a different model. And QModelIndex::isValid() does not check that.

Giuseppe D'Angelo says:

I personally maintain quite a strong point of view about this issue: passing such indices is a violation of the API contract. A model should never be assumed to be able to handle illegal indices. In other words, in my (not so humble) opinion, the QAbstractItemModel API has a narrow contract.

But since All Programmers Make Mistakes (TM), it facilitates the debugging process to check indices for being well-formed. For this purpose, Giuseppe D'Angelo introduced QAbstractItemModel::checkIndex() into Qt 5.11.

If you're still using a lower Qt version, you can simply write that function yourself.

Answer 2

[ If anyone has a better answer, I'll gladly accept it. ]

Looking at the source for QListWidget, it seems that checking the inputs is what Qt does itself:

QModelIndex QListModel::index(int row, int column, const QModelIndex &parent) const
{
    if (hasIndex(row, column, parent))
        return createIndex(row, column, items.at(row));
    return QModelIndex();
}

It also appears that I didn't know about hasIndex() which will do what my validRowColumn() method does.

bool QAbstractItemModel::hasIndex(int row, int column, const QModelIndex &parent) const
{
    if (row < 0 || column < 0)
        return false;
    return row < rowCount(parent) && column < columnCount(parent);
}

For that matter, I'm not sure why the documentation uses index.isValid() everywhere when hasIndex(index.row(), index.column(), index.parent()) would be more appropriate. Then, I'm sure a hasIndex(QModelIndex &) method would be added. hasIndex() does the same check as QModelIndex::isValid() and more:

inline bool isValid() const { return (r >= 0) && (c >= 0) && (m != 0); }