Differential D21525 Diff 59227 src/models/datamodel.cpp

Changeset View

Standalone View

View Options

src/models/datamodel.cpp

Show All 28 Lines
29#include <algorithm> 29​#include <algorithm>
30 30​
31class DataModelPrivate 31​class DataModelPrivate
32{ 32​{
33public: 33​public:
34 34​
35 DataModel::ListTrackDataType mAllTrackData; 35​ DataModel::ListTrackDataType mAllTrackData;
36 36​
37​ DataModel::ListTrackDataType mAllRadiosData;
mgallienUnsubmitted
Done

It would better (safer) if you add one more type like DataModel::ListRadioDataType. Online radios are similar to tracks but by doing that, the compiler will help avoid mistakes when connecting signals to slots.

mgallien: It would better (safer) if you add one more type like DataModel::ListRadioDataType. Online…
38​
37 DataModel::ListAlbumDataType mAllAlbumData; 39​ DataModel::ListAlbumDataType mAllAlbumData;
38 40​
39 DataModel::ListArtistDataType mAllArtistData; 41​ DataModel::ListArtistDataType mAllArtistData;
40 42​
41 DataModel::ListGenreDataType mAllGenreData; 43​ DataModel::ListGenreDataType mAllGenreData;
42 44​
43 ModelDataLoader mDataLoader; 45​ ModelDataLoader mDataLoader;
44 46​
Show All 40 Lines86{
85 auto roles = QAbstractListModel::roleNames(); 87​ auto roles = QAbstractListModel::roleNames();
86 88​
87 roles[static_cast<int>(DatabaseInterface::ColumnsRoles::TitleRole)] = "title"; 89​ roles[static_cast<int>(DatabaseInterface::ColumnsRoles::TitleRole)] = "title";
88 roles[static_cast<int>(DatabaseInterface::ColumnsRoles::SecondaryTextRole)] = "secondaryText"; 90​ roles[static_cast<int>(DatabaseInterface::ColumnsRoles::SecondaryTextRole)] = "secondaryText";
89 roles[static_cast<int>(DatabaseInterface::ColumnsRoles::ImageUrlRole)] = "imageUrl"; 91​ roles[static_cast<int>(DatabaseInterface::ColumnsRoles::ImageUrlRole)] = "imageUrl";
90 roles[static_cast<int>(DatabaseInterface::ColumnsRoles::DatabaseIdRole)] = "databaseId"; 92​ roles[static_cast<int>(DatabaseInterface::ColumnsRoles::DatabaseIdRole)] = "databaseId";
91 roles[static_cast<int>(DatabaseInterface::ColumnsRoles::ElementTypeRole)] = "dataType"; 93​ roles[static_cast<int>(DatabaseInterface::ColumnsRoles::ElementTypeRole)] = "dataType";
92 94​
95​ roles[static_cast<int>(DatabaseInterface::ColumnsRoles::HttpAddressRole)] = "httpAddress";
96​
93 roles[static_cast<int>(DatabaseInterface::ColumnsRoles::ArtistRole)] = "artist"; 97​ roles[static_cast<int>(DatabaseInterface::ColumnsRoles::ArtistRole)] = "artist";
94 roles[static_cast<int>(DatabaseInterface::ColumnsRoles::AllArtistsRole)] = "allArtists"; 98​ roles[static_cast<int>(DatabaseInterface::ColumnsRoles::AllArtistsRole)] = "allArtists";
95 roles[static_cast<int>(DatabaseInterface::ColumnsRoles::HighestTrackRating)] = "highestTrackRating"; 99​ roles[static_cast<int>(DatabaseInterface::ColumnsRoles::HighestTrackRating)] = "highestTrackRating";
96 roles[static_cast<int>(DatabaseInterface::ColumnsRoles::GenreRole)] = "genre"; 100​ roles[static_cast<int>(DatabaseInterface::ColumnsRoles::GenreRole)] = "genre";
97 101​
98 roles[static_cast<int>(DatabaseInterface::ColumnsRoles::AlbumRole)] = "album"; 102​ roles[static_cast<int>(DatabaseInterface::ColumnsRoles::AlbumRole)] = "album";
99 roles[static_cast<int>(DatabaseInterface::ColumnsRoles::AlbumArtistRole)] = "albumArtist"; 103​ roles[static_cast<int>(DatabaseInterface::ColumnsRoles::AlbumArtistRole)] = "albumArtist";
100 roles[static_cast<int>(DatabaseInterface::ColumnsRoles::DurationRole)] = "duration"; 104​ roles[static_cast<int>(DatabaseInterface::ColumnsRoles::DurationRole)] = "duration";
Show All 17 Lines
118QVariant DataModel::data(const QModelIndex &index, int role) const 122​QVariant DataModel::data(const QModelIndex &index, int role) const
119{ 123​{
120 auto result = QVariant(); 124​ auto result = QVariant();
121 125​
122 const auto dataCount = d->mAllTrackData.size() + d->mAllAlbumData.size() + d->mAllArtistData.size() + d->mAllGenreData.size(); 126​ const auto dataCount = d->mAllTrackData.size() + d->mAllAlbumData.size() + d->mAllArtistData.size() + d->mAllGenreData.size();
123 127​
124 Q_ASSERT(index.isValid()); 128​ Q_ASSERT(index.isValid());
125 Q_ASSERT(index.column() == 0); 129​ Q_ASSERT(index.column() == 0);
126 Q_ASSERT(index.row() >= 0 && index.row() < dataCount); ​
127 Q_ASSERT(!index.parent().isValid()); 130​ Q_ASSERT(!index.parent().isValid());
128 Q_ASSERT(index.model() == this); 131​ Q_ASSERT(index.model() == this);
129 Q_ASSERT(index.internalId() == 0); 132​ Q_ASSERT(index.internalId() == 0);
130 133​
134​ if(d->mModelType != ElisaUtils::Radio){
135​ Q_ASSERT(index.row() >= 0 && index.row() < dataCount);
136​ }
137​
mgallienUnsubmitted
Not Done

What is the reason for that ?
It does not look good.

mgallien: What is the reason for that ? It does not look good.
jguidonAuthorUnsubmitted
Not Done

The dataCount did not make sense for radios, I reintegrated the check and added a different dataCount on line 128.

jguidon: The dataCount did not make sense for radios, I reintegrated the check and added a different…
131 switch(role) 138​ switch(role)
132 { 139​ {
133 case Qt::DisplayRole: 140​ case Qt::DisplayRole:
134 switch(d->mModelType) 141​ switch(d->mModelType)
135 { 142​ {
136 case ElisaUtils::Track: 143​ case ElisaUtils::Track:
137 result = d->mAllTrackData[index.row()][TrackDataType::key_type::TitleRole]; 144​ result = d->mAllTrackData[index.row()][TrackDataType::key_type::TitleRole];
138 break; 145​ break;
139 case ElisaUtils::Album: 146​ case ElisaUtils::Album:
140 result = d->mAllAlbumData[index.row()][AlbumDataType::key_type::TitleRole]; 147​ result = d->mAllAlbumData[index.row()][AlbumDataType::key_type::TitleRole];
141 break; 148​ break;
142 case ElisaUtils::Artist: 149​ case ElisaUtils::Artist:
143 result = d->mAllArtistData[index.row()][ArtistDataType::key_type::TitleRole]; 150​ result = d->mAllArtistData[index.row()][ArtistDataType::key_type::TitleRole];
144 break; 151​ break;
145 case ElisaUtils::Genre: 152​ case ElisaUtils::Genre:
146 result = d->mAllGenreData[index.row()][GenreDataType::key_type::TitleRole]; 153​ result = d->mAllGenreData[index.row()][GenreDataType::key_type::TitleRole];
147 break; 154​ break;
155​ case ElisaUtils::Radio:
156​ result = d->mAllRadiosData[index.row()][GenreDataType::key_type::TitleRole];
157​ break;
148 case ElisaUtils::Lyricist: 158​ case ElisaUtils::Lyricist:
149 case ElisaUtils::Composer: 159​ case ElisaUtils::Composer:
150 case ElisaUtils::FileName: 160​ case ElisaUtils::FileName:
151 case ElisaUtils::Unknown: 161​ case ElisaUtils::Unknown:
152 break; 162​ break;
153 } 163​ }
154 break; 164​ break;
155 case DatabaseInterface::ColumnsRoles::DurationRole: 165​ case DatabaseInterface::ColumnsRoles::DurationRole:
156 { 166​ {
157 if (d->mModelType == ElisaUtils::Track) { 167​ if (d->mModelType == ElisaUtils::Track) {
158 auto trackDuration = d->mAllTrackData[index.row()][TrackDataType::key_type::DurationRole].toTime(); 168​ auto trackDuration = d->mAllTrackData[index.row()][TrackDataType::key_type::DurationRole].toTime();
159 if (trackDuration.hour() == 0) { 169​ if (trackDuration.hour() == 0) {
160 result = trackDuration.toString(QStringLiteral("mm:ss")); 170​ result = trackDuration.toString(QStringLiteral("mm:ss"));
161 } else { 171​ } else {
162 result = trackDuration.toString(); 172​ result = trackDuration.toString();
163 } 173​ }
164 } 174​ }
165 break; 175​ break;
166 } 176​ }
167 default: 177​ default:
astippichUnsubmitted
Done

use QString instead of QStringLiteral(""), it is a little bit cheaper

astippich: use QString instead of QStringLiteral(""), it is a little bit cheaper
mgallienUnsubmitted
Done

No need for that. If the data is missing, the view should handle that (i.e. an empty/invalid QVariant). This is the same in most places where fallback is an empty/invalid QVariant when the data is missing or unavailable.

mgallien: No need for that. If the data is missing, the view should handle that (i.e. an empty/invalid…
168 switch(d->mModelType) 178​ switch(d->mModelType)
169 { 179​ {
170 case ElisaUtils::Track: 180​ case ElisaUtils::Track:
171 result = d->mAllTrackData[index.row()][static_cast<TrackDataType::key_type>(role)]; 181​ result = d->mAllTrackData[index.row()][static_cast<TrackDataType::key_type>(role)];
172 break; 182​ break;
173 case ElisaUtils::Album: 183​ case ElisaUtils::Album:
174 result = d->mAllAlbumData[index.row()][static_cast<AlbumDataType::key_type>(role)]; 184​ result = d->mAllAlbumData[index.row()][static_cast<AlbumDataType::key_type>(role)];
175 break; 185​ break;
176 case ElisaUtils::Artist: 186​ case ElisaUtils::Artist:
177 result = d->mAllArtistData[index.row()][static_cast<ArtistDataType::key_type>(role)]; 187​ result = d->mAllArtistData[index.row()][static_cast<ArtistDataType::key_type>(role)];
178 break; 188​ break;
179 case ElisaUtils::Genre: 189​ case ElisaUtils::Genre:
180 result = d->mAllGenreData[index.row()][static_cast<GenreDataType::key_type>(role)]; 190​ result = d->mAllGenreData[index.row()][static_cast<GenreDataType::key_type>(role)];
181 break; 191​ break;
192​ case ElisaUtils::Radio:
193​ result = d->mAllRadiosData[index.row()][static_cast<TrackDataType::key_type>(role)];
194​ break;
182 case ElisaUtils::Lyricist: 195​ case ElisaUtils::Lyricist:
183 case ElisaUtils::Composer: 196​ case ElisaUtils::Composer:
184 case ElisaUtils::FileName: 197​ case ElisaUtils::FileName:
185 case ElisaUtils::Unknown: 198​ case ElisaUtils::Unknown:
186 break; 199​ break;
187 } 200​ }
188 } 201​ }
189 202​
▲ Show 20 Lines • Show All 200 Lines • ▼ Show 20 Line(s)400 for (result = 0; result < d->mAllTrackData.size(); ++result) {
390 } 403​ }
391 } 404​ }
392 405​
393 result = -1; 406​ result = -1;
394 407​
395 return result; 408​ return result;
396} 409​}
397 410​
398void DataModel::connectModel(DatabaseInterface *database) 411​void DataModel::connectModel(DatabaseInterface *database)
399{ 412​{
400 d->mDataLoader.setDatabase(database); 413​ d->mDataLoader.setDatabase(database);
401 414​
402 connect(&d->mDataLoader, &ModelDataLoader::allTracksData, 415​ connect(&d->mDataLoader, &ModelDataLoader::allTracksData,
403 this, &DataModel::tracksAdded); 416​ this, &DataModel::tracksAdded);
417​ connect(&d->mDataLoader, &ModelDataLoader::allRadiosData,
418​ this, &DataModel::radiosAdded);
404 connect(&d->mDataLoader, &ModelDataLoader::allAlbumsData, 419​ connect(&d->mDataLoader, &ModelDataLoader::allAlbumsData,
405 this, &DataModel::albumsAdded); 420​ this, &DataModel::albumsAdded);
406 connect(&d->mDataLoader, &ModelDataLoader::allArtistsData, 421​ connect(&d->mDataLoader, &ModelDataLoader::allArtistsData,
407 this, &DataModel::artistsAdded); 422​ this, &DataModel::artistsAdded);
408 connect(&d->mDataLoader, &ModelDataLoader::allGenresData, 423​ connect(&d->mDataLoader, &ModelDataLoader::allGenresData,
409 this, &DataModel::genresAdded); 424​ this, &DataModel::genresAdded);
410 connect(&d->mDataLoader, &ModelDataLoader::genresAdded, 425​ connect(&d->mDataLoader, &ModelDataLoader::genresAdded,
mgallienUnsubmitted
Done

This is too much specific to one usecase of DataModel.
Why do you need this when other views do not need it ?

mgallien: This is too much specific to one usecase of DataModel. Why do you need this when other views do…
jguidonAuthorUnsubmitted
Done

I just copied trackIndexFromId, in the first case we get mAllTrackData[result] and mAllRadiosData[result] in the other. I will integrate both methods.

jguidon: I just copied trackIndexFromId, in the first case we get mAllTrackData[result] and…
411 this, &DataModel::genresAdded); 426​ this, &DataModel::genresAdded);
412 connect(&d->mDataLoader, &ModelDataLoader::albumsAdded, 427​ connect(&d->mDataLoader, &ModelDataLoader::albumsAdded,
413 this, &DataModel::albumsAdded); 428​ this, &DataModel::albumsAdded);
414 connect(&d->mDataLoader, &ModelDataLoader::albumModified, 429​ connect(&d->mDataLoader, &ModelDataLoader::albumModified,
415 this, &DataModel::albumModified); 430​ this, &DataModel::albumModified);
416 connect(&d->mDataLoader, &ModelDataLoader::albumRemoved, 431​ connect(&d->mDataLoader, &ModelDataLoader::albumRemoved,
417 this, &DataModel::albumRemoved); 432​ this, &DataModel::albumRemoved);
418 connect(&d->mDataLoader, &ModelDataLoader::tracksAdded, 433​ connect(&d->mDataLoader, &ModelDataLoader::tracksAdded,
▲ Show 20 Lines • Show All 64 Lines • ▼ Show 20 Line(s)491 } else {
483 } else { 498​ } else {
484 beginInsertRows({}, d->mAllTrackData.size(), d->mAllTrackData.size() + newData.size() - 1); 499​ beginInsertRows({}, d->mAllTrackData.size(), d->mAllTrackData.size() + newData.size() - 1);
485 d->mAllTrackData.append(newData); 500​ d->mAllTrackData.append(newData);
486 endInsertRows(); 501​ endInsertRows();
487 } 502​ }
488 } 503​ }
489} 504​}
490 505​
506​void DataModel::radiosAdded(ListTrackDataType newData)
507​{
508​ if (newData.isEmpty() && d->mModelType == ElisaUtils::Radio) {
509​ setBusy(false);
510​ }
511​
512​ if (newData.isEmpty() || d->mModelType != ElisaUtils::Radio) {
513​ return;
514​ }
515​
516​ if (d->mFilterType == FilterById && !d->mAllRadiosData.isEmpty()) {
517​ for (const auto &newTrack : newData) {
518​ auto trackIndex = trackIndexFromId(newTrack.databaseId());
519​
520​ if (trackIndex != -1) {
521​ continue;
522​ }
523​
524​ bool trackInserted = false;
525​ for (int trackIndex = 0; trackIndex < d->mAllRadiosData.count(); ++trackIndex) {
526​ const auto &oneTrack = d->mAllRadiosData[trackIndex];
527​
528​ if (oneTrack.trackNumber() > newTrack.trackNumber()) {
529​ beginInsertRows({}, trackIndex, trackIndex);
530​ d->mAllRadiosData.insert(trackIndex, newTrack);
531​ endInsertRows();
532​
533​ if (d->mAllRadiosData.size() == 1) {
534​ setBusy(false);
535​ }
536​
537​ trackInserted = true;
538​ break;
539​ }
540​ }
541​
542​ if (!trackInserted) {
543​ beginInsertRows({}, d->mAllRadiosData.count(), d->mAllRadiosData.count());
544​ d->mAllRadiosData.insert(d->mAllRadiosData.count(), newTrack);
545​ endInsertRows();
546​
547​ if (d->mAllRadiosData.size() == 1) {
548​ setBusy(false);
549​ }
550​ }
551​ }
552​ } else {
553​ if (d->mAllRadiosData.isEmpty()) {
554​ beginInsertRows({}, 0, newData.size() - 1);
555​ d->mAllRadiosData.swap(newData);
556​ endInsertRows();
557​
558​ setBusy(false);
559​ } else {
560​ beginInsertRows({}, d->mAllRadiosData.size(), d->mAllRadiosData.size() + newData.size() - 1);
561​ d->mAllRadiosData.append(newData);
562​ endInsertRows();
563​ }
564​ }
565​}
566​
491void DataModel::trackModified(const TrackDataType &modifiedTrack) 567​void DataModel::trackModified(const TrackDataType &modifiedTrack)
492{ 568​{
493 if (d->mModelType != ElisaUtils::Track) { 569​ if (d->mModelType != ElisaUtils::Track) {
494 return; 570​ return;
495 } 571​ }
496 572​
497 if (!d->mAlbumTitle.isEmpty() && !d->mAlbumArtist.isEmpty()) { 573​ if (!d->mAlbumTitle.isEmpty() && !d->mAlbumArtist.isEmpty()) {
498 if (modifiedTrack.album() != d->mAlbumTitle) { 574​ if (modifiedTrack.album() != d->mAlbumTitle) {
▲ Show 20 Lines • Show All 209 Lines • Show Last 20 Lines