QList.removeAt(int index) removes wrong item

[vlada]

Hi,

I'm getting totally mad. For maybe 5 hours I'm trying to fix my app but I still can't find the problem. I use a QAbstractListModel to show a list in my app. If I delete one item from the list, usually wrong item gets deleted. I absolutely don't understand it. Furthermore the behavior is not deterministic. Sometimes the right item gets deleted, but most of the time it is the next one.

This is my source code:
manageplaylistsmodel.h

#ifndef MANAGEPLAYLISTSMODEL_H
#define MANAGEPLAYLISTSMODEL_H
#include <QAbstractListModel>
#include <QStringList>
#include <QDebug>

class ManagePlaylistsItem
{
public:
    ManagePlaylistsItem(const QString &title, const QString &source, const bool &dynamic);

    QString title() const;
    QString source() const;
    bool dynamic() const;
    void setTitle(QString title);
    void setSource(QString source);
    void modifyPlaylist(const QString &name, const QString &source);

private:
    QString m_title;
    QString m_source;
    bool  m_dynamic;
};

class ManagePlaylistsModel : public QAbstractListModel
{
    Q_OBJECT
public:
    enum LibraryRoles {
        TitleRole = Qt::UserRole + 1,
        SourceRole,
        DynamicRole,
    };

    ManagePlaylistsModel(QObject *parent = nullptr);
    bool setData(const QModelIndex &index, const QVariant &value, int role) override;

    void modifyPlaylist(const int &index, const QString &name, const QString &source);

    void addItem(const ManagePlaylistsItem &item);

    bool removeRow(const int row, const QModelIndex &parent = QModelIndex());

    bool removeRows(const int row, int count, const QModelIndex &parent = QModelIndex()) override;

    void removeAllItems(const QModelIndex &parent = QModelIndex());

    void moveRow(const int index1, const int index2);

    int rowCount(const QModelIndex & parent = QModelIndex()) const override;

    int indexOf(const QString &name);

    QString getName(const int index);

    QString getQuery(const int index);

    bool getType(const int index);

    Qt::ItemFlags flags(const QModelIndex &index) const override;

    QVariant data(const QModelIndex & index, int role = Qt::DisplayRole) const override;

    QHash<int,QByteArray> roleNames() const override;


private:
    QList<ManagePlaylistsItem> m_items;
};

#endif // MANAGEPLAYLISTSMODEL_H

manageplaylistsmodel.cpp

#include "manageplaylistsmodel.h"

ManagePlaylistsItem::ManagePlaylistsItem(const QString &title, const QString &source, const bool &dynamic)
    : m_title(title), m_source(source), m_dynamic(dynamic)
{
}

QString ManagePlaylistsItem::title() const
{
    return m_title;
}

void ManagePlaylistsItem::setTitle(QString title) {
    m_title = title;
}

QString ManagePlaylistsItem::source() const
{
    return m_source;
}

void ManagePlaylistsItem::setSource(QString source) {
    m_source = source;
}

bool ManagePlaylistsItem::dynamic() const
{
    return m_dynamic;
}

ManagePlaylistsModel::ManagePlaylistsModel(QObject *parent)
    : QAbstractListModel(parent)
{
}

bool ManagePlaylistsModel::setData(const QModelIndex &index, const QVariant &value, int role)
{
    if (!hasIndex(index.row(), index.column(), index.parent()) || !value.isValid())
        return false;

    ManagePlaylistsItem &item = m_items[index.row()];
    if (role == TitleRole) item.setTitle(value.toString());
    else if (role == SourceRole) item.setSource(value.toString());
    else return false;

    emit dataChanged(index, index, { role } );
    return true ;

}

void ManagePlaylistsModel::modifyPlaylist(const int &itemIndex, const QString &name, const QString &source) {
    QModelIndex index = this->index(itemIndex, 0, QModelIndex());
    if (m_items[itemIndex].title() != name && name != "")
        setData(index, name, TitleRole);
    if (m_items[itemIndex].source() != source && source != "") {
        if (source == " ")
            setData(index, "", SourceRole);
        else
            setData(index, source, SourceRole);
    }
}

int ManagePlaylistsModel::indexOf(const QString &name) {
    int itemIndex = -1;
    for (int i=0; i<rowCount(); i++) {
        if (m_items[i].title() == name) {
            itemIndex = i;
            break;
        }
    }
    return itemIndex;
}

QString ManagePlaylistsModel::getName(const int index) {
    if (index == -1)
        return("<ALL>");
    return m_items[index].title();
}

QString ManagePlaylistsModel::getQuery(const int index) {
    if (index == -1)
        return("");
    return m_items[index].source();
}


bool ManagePlaylistsModel::getType(const int index) {
    if (index == -1)
        return(true);
    return m_items[index].dynamic();
}

void ManagePlaylistsModel::addItem(const ManagePlaylistsItem &item)
{
    beginInsertRows(QModelIndex(), rowCount(), rowCount());
    m_items << item;
    endInsertRows();
}

bool ManagePlaylistsModel::removeRow(const int row, const QModelIndex &) {
    return removeRows(row, 1);
}

bool ManagePlaylistsModel::removeRows(const int row, int count, const QModelIndex &) {
    for (int i=0;i<rowCount();i++) {
    }
    beginRemoveRows(QModelIndex(), row, row + count - 1);
    while (count--) m_items.removeAt(row);
    endRemoveRows();
    return true;
}

void ManagePlaylistsModel::removeAllItems(const QModelIndex &) {
    if (rowCount() > 0)
        removeRows(0, rowCount());
}

void ManagePlaylistsModel::moveRow(const int index1, const int index2) {
    beginMoveRows(QModelIndex(), index1, index1, QModelIndex(), index2);
    m_items.swap(index1, index2);
    endMoveRows();
}

int ManagePlaylistsModel::rowCount(const QModelIndex &parent) const {
    if (parent.isValid())
        return 0;
    return m_items.count();
}

Qt::ItemFlags ManagePlaylistsModel::flags(const QModelIndex &index) const
{
    if (!index.isValid())
        return Qt::ItemIsEnabled;
    return QAbstractListModel::flags(index) | Qt::ItemIsEditable;
}

QVariant ManagePlaylistsModel::data(const QModelIndex & index, int role) const {
    if (index.row() < 0 || index.row() >= m_items.count())
        return QVariant();
    const ManagePlaylistsItem &item = m_items[index.row()];
    if (role == TitleRole)
        return item.title();
    else if (role == SourceRole)
        return item.source();
    else if (role == DynamicRole)
        return item.dynamic();
    return QVariant();
}

QHash<int, QByteArray> ManagePlaylistsModel::roleNames() const
{
    QHash<int, QByteArray> roles;
    roles[TitleRole] = "title";
    roles[SourceRole] = "source";
    roles[DynamicRole] = "dynamic";
    return roles;
}

Do you have any idea what might be wrong here? I use another 3 QAbstractListModels in my app and they all work flawlessly. I can't see any significant difference between them.

Thank you.

[Christian Ehrlicher]

You should check what index you pass to removeRow() - I would guess you're passing a wrong one there. QList::removeAt() is for sure removing the correct one (== the index you're passing in)

[vlada]

I'm think I must be crazy. Yesterday I checked that I'm passing the correct index maybe hundred times. I spent many hours debugging the code. Now I just added some debug outputs to demonstrate the problem and without changing any real code, it works just as expected. I don't understand it...

[JonB]

@vlada said in QList.removeAt(int index) removes wrong item:

I don't understand it...

You were indeed going crazy :)