Once more : QAbstractItemModel woes

[Valheru]

I am trying to implement a QAbstractItemModel/QTreeView MVC in a program. I have my data in the form of QLists - a list of albums, and every album contains a list of tracks.

Everything is working, except my data() implementation never returns the right data for the children (tracks). Instead, it always returns the data for the albums, with the result that when the albums expand they list the albums yet again. There is no infinite recursion - i.e. the rowCount() implementation works. I also checked with debug prints to the console that the index() and parent() methods work as expected.

Here is the code for the model implementation:

/***************************************************************************
 *   Copyright (C) 2007 by Lawrence Lee   *
 *   valheru@facticius.net   *
 *                                                                         *
 *   This program is free software; you can redistribute it and/or modify  *
 *   it under the terms of the GNU General Public License as published by  *
 *   the Free Software Foundation; either version 2 of the License, or     *
 *   (at your option) any later version.                                   *
 *                                                                         *
 *   This program is distributed in the hope that it will be useful,       *
 *   but WITHOUT ANY WARRANTY; without even the implied warranty of        *
 *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the         *
 *   GNU General Public License for more details.                          *
 *                                                                         *
 *   You should have received a copy of the GNU General Public License     *
 *   along with this program; if not, write to the                         *
 *   Free Software Foundation, Inc.,                                       *
 *   59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.             *
 ***************************************************************************/
#include <KDE/KDebug>
#include "kzenalbumviewmodel.h"
#include "kzenalbum.h"
#include "kzentrack.h"
 
KZenAlbumViewModel::KZenAlbumViewModel( QObject *parent, const QList<KZenAlbum*> &albums )
 : QAbstractItemModel( parent ), albumItems( albums )
{
    rootItem << "Album" << "Artist" << "Genre" << "Nr. of Tracks";
}
 
KZenAlbumViewModel::~KZenAlbumViewModel()
{
    qDeleteAll( albumItems );
}
 
int KZenAlbumViewModel::columnCount( const QModelIndex& /*parent*/ ) const
{
    return 4;
}
 
QVariant KZenAlbumViewModel::data( const QModelIndex &index, int role ) const
{
    kDebug() << index;
 
    if( !index.isValid() )
        return QVariant();
 
    if( role != Qt::DisplayRole )
        return QVariant();
 
    QObject *item = static_cast<QObject*>( index.internalPointer() );
 
    if( item ){
        KZenAlbum *album = qobject_cast<KZenAlbum*>( item );
 
        if( album ){ //Album
 
            switch( index.column() ){
                case 0:
                    return album->name();
                case 1:
                    return album->artist();
                case 2:
                    return album->genre();
                case 3:
                    return album->numTracks();
                default:
                    return QVariant();
            }
        }else{ //Track
            KZenTrack *track = qobject_cast<KZenTrack*>( item );
 
            if( track ){
                switch( index.column() ){
                    case 0:
                        return track->title();
                    case 1:
                        return track->artist();
                    case 2:
                        return track->genre();
                    case 3:
                        return track->tracknumber();
                    default:
                        return QVariant();
                }
            }else{
                return QVariant();
            }
        }
    }
 
    return QVariant();
}
 
Qt::ItemFlags KZenAlbumViewModel::flags( const QModelIndex &index ) const
{
    if( !index.isValid() )
        return 0;
 
    return Qt::ItemIsEnabled | Qt::ItemIsSelectable;
}
 
QVariant KZenAlbumViewModel::headerData( int section, Qt::Orientation orientation, int role ) const
{
    if (orientation == Qt::Horizontal && role == Qt::DisplayRole){
        return rootItem.value( section );
    }
 
    return QVariant();
}
 
QModelIndex KZenAlbumViewModel::index( int row, int column, const QModelIndex &parent ) const
{
    if( !hasIndex( row, column, parent ) )
        return QModelIndex();
 
    if( !parent.isValid() ){ // This is an album, and therefor a top-level item
        KZenAlbum *album = const_cast<KZenAlbum*>( albumItems.at( row ) );
 
        if( album )
            return createIndex( row, column, album );
        else
            return QModelIndex();
 
    }else{ //This is a track, and has an album as a parent
        QObject *item = static_cast<QObject*>( parent.internalPointer() );
        KZenAlbum *album = qobject_cast<KZenAlbum*>( item );
        KZenTrack *track = album->albumTracks().value( row );
        return createIndex( row, column, track );
    }
 
    return QModelIndex();
}
 
QModelIndex KZenAlbumViewModel::parent( const QModelIndex &index ) const
{
    if( !index.isValid() )
        return QModelIndex();
 
    QObject *item = static_cast<QObject*>( index.internalPointer() );
 
    if( item ){
        KZenAlbum *album = qobject_cast<KZenAlbum*>( item );
 
        if( album ){
            return QModelIndex();
        }else{
            KZenTrack *track = qobject_cast<KZenTrack*>( item );
 
            if( track ){
                KZenAlbum *album = track->parent();
                return createIndex( albumItems.indexOf( album ), 0, album );
            }else{
                return QModelIndex();
            }
        }
    }
 
    return QModelIndex();
}
 
int KZenAlbumViewModel::rowCount( const QModelIndex &parent ) const
{
    if( parent.column() > 0 )
        //Out of model bounds
        return 0;
 
    if( !parent.isValid() )
        //We are in the root of the model
        return albumItems.size();
 
    //Check to see if this is a track
    QObject *item = static_cast<QObject*>( parent.internalPointer() );
 
    if( item ){
        KZenAlbum *album = qobject_cast<KZenAlbum*>( item );
 
        if( album ){
            return album->numTracks();
        }else{
            return 0;
        }
    }
 
    return 0;
 
}
 
#include "kzenalbumviewmodel.moc"

/***************************************************************************
 *   Copyright (C) 2007 by Lawrence Lee   *
 *   valheru@facticius.net   *
 *                                                                         *
 *   This program is free software; you can redistribute it and/or modify  *
 *   it under the terms of the GNU General Public License as published by  *
 *   the Free Software Foundation; either version 2 of the License, or     *
 *   (at your option) any later version.                                   *
 *                                                                         *
 *   This program is distributed in the hope that it will be useful,       *
 *   but WITHOUT ANY WARRANTY; without even the implied warranty of        *
 *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the         *
 *   GNU General Public License for more details.                          *
 *                                                                         *
 *   You should have received a copy of the GNU General Public License     *
 *   along with this program; if not, write to the                         *
 *   Free Software Foundation, Inc.,                                       *
 *   59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.             *
 ***************************************************************************/
#include <KDE/KDebug>
#include "kzenalbumviewmodel.h"
#include "kzenalbum.h"
#include "kzentrack.h"

KZenAlbumViewModel::KZenAlbumViewModel( QObject *parent, const QList<KZenAlbum*> &albums )
 : QAbstractItemModel( parent ), albumItems( albums )
{
    rootItem << "Album" << "Artist" << "Genre" << "Nr. of Tracks";
}

KZenAlbumViewModel::~KZenAlbumViewModel()
{
    qDeleteAll( albumItems );
}

int KZenAlbumViewModel::columnCount( const QModelIndex& /*parent*/ ) const
{
    return 4;
}

QVariant KZenAlbumViewModel::data( const QModelIndex &index, int role ) const
{
    kDebug() << index;

    if( !index.isValid() )
        return QVariant();

    if( role != Qt::DisplayRole )
        return QVariant();

    QObject *item = static_cast<QObject*>( index.internalPointer() );

    if( item ){
        KZenAlbum *album = qobject_cast<KZenAlbum*>( item );

        if( album ){ //Album

            switch( index.column() ){
                case 0:
                    return album->name();
                case 1:
                    return album->artist();
                case 2:
                    return album->genre();
                case 3:
                    return album->numTracks();
                default:
                    return QVariant();
            }
        }else{ //Track
            KZenTrack *track = qobject_cast<KZenTrack*>( item );

            if( track ){
                switch( index.column() ){
                    case 0:
                        return track->title();
                    case 1:
                        return track->artist();
                    case 2:
                        return track->genre();
                    case 3:
                        return track->tracknumber();
                    default:
                        return QVariant();
                }
            }else{
                return QVariant();
            }
        }
    }

    return QVariant();
}

Qt::ItemFlags KZenAlbumViewModel::flags( const QModelIndex &index ) const
{
    if( !index.isValid() )
        return 0;

    return Qt::ItemIsEnabled | Qt::ItemIsSelectable;
}

QVariant KZenAlbumViewModel::headerData( int section, Qt::Orientation orientation, int role ) const
{
    if (orientation == Qt::Horizontal && role == Qt::DisplayRole){
        return rootItem.value( section );
    }

    return QVariant();
}

QModelIndex KZenAlbumViewModel::index( int row, int column, const QModelIndex &parent ) const
{
    if( !hasIndex( row, column, parent ) )
        return QModelIndex();

    if( !parent.isValid() ){ // This is an album, and therefor a top-level item
        KZenAlbum *album = const_cast<KZenAlbum*>( albumItems.at( row ) );

        if( album )
            return createIndex( row, column, album );
        else
            return QModelIndex();

    }else{ //This is a track, and has an album as a parent
        QObject *item = static_cast<QObject*>( parent.internalPointer() );
        KZenAlbum *album = qobject_cast<KZenAlbum*>( item );
        KZenTrack *track = album->albumTracks().value( row );
        return createIndex( row, column, track );
    }

    return QModelIndex();
}

QModelIndex KZenAlbumViewModel::parent( const QModelIndex &index ) const
{
    if( !index.isValid() )
        return QModelIndex();

    QObject *item = static_cast<QObject*>( index.internalPointer() );

    if( item ){
        KZenAlbum *album = qobject_cast<KZenAlbum*>( item );

        if( album ){
            return QModelIndex();
        }else{
            KZenTrack *track = qobject_cast<KZenTrack*>( item );

            if( track ){
                KZenAlbum *album = track->parent();
                return createIndex( albumItems.indexOf( album ), 0, album );
            }else{
                return QModelIndex();
            }
        }
    }

    return QModelIndex();
}

int KZenAlbumViewModel::rowCount( const QModelIndex &parent ) const
{
    if( parent.column() > 0 )
        //Out of model bounds
        return 0;

    if( !parent.isValid() )
        //We are in the root of the model
        return albumItems.size();

    //Check to see if this is a track
    QObject *item = static_cast<QObject*>( parent.internalPointer() );

    if( item ){
        KZenAlbum *album = qobject_cast<KZenAlbum*>( item );

        if( album ){
            return album->numTracks();
        }else{
            return 0;
        }
    }

    return 0;

}

#include "kzenalbumviewmodel.moc"

To copy to clipboard, switch view to plain text mode 

[wysota]

Why are your albums and tracks QObjects?

[Valheru]

Why does that matter?

[wysota]

It's my curiosity. I doubt it has meanings other than the performance hit. But it's also another layer you add to the complexity increasing total "proness" to errors. Simplifing your code would make it more robust and easier to read, effectively making it easier to spot the real problem. Currently all the casts you have make it quite difficult to follow, especially not knowing how your items look like.

BTW. Have you tried ModelTest from labs.trolltech.com?

[Valheru]

Sorry, I had tried ModelTest. I've solved the problem - the cause was that I was tracking the tracks parent internally in the Track class, and was calling the QObjects parent() method to get the tracks parent - which was returning NULL. Once I got rid of that it all worked. If you look at the code you will see that I was returning a new QModelIndex if the parent() call failed - which is why the recursion was happening.

The albums and tracks need to be QObjects because the user can edit information in the program about the files on their MP3 player, and the objects need to keep track of any editing going on in the views. I thought at the beginning that it would be easiest to do this using signals, although I now think it may be better to use the QModelIndex of the edited item to achieve this. Although I am unsure how complex this would get if I enabled sorting in the views...

[wysota]

The albums and tracks need to be QObjects because the user can edit information in the program about the files on their MP3 player, and the objects need to keep track of any editing going on in the views.

This is what the model-view architecture is for, not QObjects. If you want the view to be coherent with the rest of the application, you should only interface the data through the model.

Read this: http://blog.wysota.eu.org/index.php/...ta-redundancy/

Although I am unsure how complex this would get if I enabled sorting in the views...

As complex as this:

QSortFilterProxyModel *proxy = new QSortFilterProxyModel(this);
proxy->setSourceModel(model);
view->setModel(proxy);

QSortFilterProxyModel *proxy = new QSortFilterProxyModel(this);
proxy->setSourceModel(model);
view->setModel(proxy);

To copy to clipboard, switch view to plain text mode 

[Valheru]

Thanks. By coincidence I'd just read up about QSortFilterProxyModel last night.

One question I do still have though - in my code I am passing the data structures between the functions like so :

KZenAlbumModel( QList<KZenAlbum*> &albums, QObject *parent = 0 ) : QAbstractItemModel( parent ), m_albums( albums )

KZenAlbumModel( QList<KZenAlbum*> &albums, QObject *parent = 0 ) : QAbstractItemModel( parent ), m_albums( albums )

To copy to clipboard, switch view to plain text mode 

My question is, is the album data being duplicated, or is a reference count just being implemented in albums? m_data is declared as :

QList<KZenAlbum*> &m_albums;

QList<KZenAlbum*> &m_albums;

To copy to clipboard, switch view to plain text mode 

[wysota]

QObject's can't be copied. If you copy the list of pointers, no data gets copied. If you modify one of the lists the objects kept within it are copied. Because the list contains pointers, only those will be copied and not the objects they point at.

[Valheru]

Ok, I am rewriting the code now so that the albums and tracks are not QObject and I am running into problems with my model again.

Since they are not QObject's any more, it is difficult to figure out how to write that parent() function of the model. index.internalPointer() returns a void pointer, so there is no way to figure out if you are dealing with an album or a track.
Calling index.parent() sends my program into an infinite loop upon the creation of the model, I don't know if this is a bug in Qt or not. According to the documentation, calling index.parent() should return an invalid QModelIndex in the case of an album, but all it does is sends my program into a loop

I am stumped, I admit. How can I figure this out? It was partly this problem in the first place that caused me to make the albums and tracks QObject, because then I could use qobject_cast to see what I was dealing with. Can I solve this cleanly, or would I have to use some workaround like providing my classes with names internally and using calls to the name() functions of the class to see what it is?

Any help would be greatly appreciated.

[wysota]

Since they are not QObject's any more, it is difficult to figure out how to write that parent() function of the model.

Have a look at the "simple tree model" example that comes with Qt.

index.internalPointer() returns a void pointer, so there is no way to figure out if you are dealing with an album or a track.

Sure there is. static_cast() to a common base class and use dynamic_cast() or some dedicated method in the base class to decide the type of the object and then cast to the proper pointer type.

[Valheru]

Sorry, that is what I meant - that is of course more or less I was doing by having declared the albums and tracks as QObjects.

And I must have read the Simple Tree Model at least a dozen times Unfortunately, it is of no help when you are doing something like this, and I suspect it is for a large part responsible for that practice that you belate on your blog - the same practice that I am trying to avoid in this program, namely that of restructuring your data into *TreeItems.

I got it working by using polymorphism and first static_casting to the base class and then using dynamic_cast to figure out what I was dealing with. I suspect that this is the only way to return a correct value for the parent() function if your root and child objects are of differing types.