Differential D21525 Diff 62677 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 20 Lines • Show All 77 Lines • ▼ Show 20 Line(s)121{
122 if (!index.isValid()) { 124​ if (!index.isValid()) {
123 return result; 125​ return result;
124 } 126​ }
125 127​
126 const auto dataCount = d->mAllTrackData.size() + d->mAllAlbumData.size() + d->mAllArtistData.size() + d->mAllGenreData.size(); 128​ const auto dataCount = d->mAllTrackData.size() + d->mAllAlbumData.size() + d->mAllArtistData.size() + d->mAllGenreData.size();
127 129​
128 Q_ASSERT(index.isValid()); 130​ Q_ASSERT(index.isValid());
129 Q_ASSERT(index.column() == 0); 131​ Q_ASSERT(index.column() == 0);
130 Q_ASSERT(index.row() >= 0 && index.row() < dataCount); ​
131 Q_ASSERT(!index.parent().isValid()); 132​ Q_ASSERT(!index.parent().isValid());
132 Q_ASSERT(index.model() == this); 133​ Q_ASSERT(index.model() == this);
133 Q_ASSERT(index.internalId() == 0); 134​ Q_ASSERT(index.internalId() == 0);
134 135​
136​ if(d->mModelType != ElisaUtils::Radio){
137​ Q_ASSERT(index.row() >= 0 && index.row() < dataCount);
138​ }
139​
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…
135 switch(role) 140​ switch(role)
136 { 141​ {
137 case Qt::DisplayRole: 142​ case Qt::DisplayRole:
138 switch(d->mModelType) 143​ switch(d->mModelType)
139 { 144​ {
140 case ElisaUtils::Track: 145​ case ElisaUtils::Track:
141 result = d->mAllTrackData[index.row()][TrackDataType::key_type::TitleRole]; 146​ result = d->mAllTrackData[index.row()][TrackDataType::key_type::TitleRole];
142 break; 147​ break;
143 case ElisaUtils::Album: 148​ case ElisaUtils::Album:
144 result = d->mAllAlbumData[index.row()][AlbumDataType::key_type::TitleRole]; 149​ result = d->mAllAlbumData[index.row()][AlbumDataType::key_type::TitleRole];
145 break; 150​ break;
146 case ElisaUtils::Artist: 151​ case ElisaUtils::Artist:
147 result = d->mAllArtistData[index.row()][ArtistDataType::key_type::TitleRole]; 152​ result = d->mAllArtistData[index.row()][ArtistDataType::key_type::TitleRole];
148 break; 153​ break;
149 case ElisaUtils::Genre: 154​ case ElisaUtils::Genre:
150 result = d->mAllGenreData[index.row()][GenreDataType::key_type::TitleRole]; 155​ result = d->mAllGenreData[index.row()][GenreDataType::key_type::TitleRole];
151 break; 156​ break;
157​ case ElisaUtils::Radio:
158​ result = d->mAllRadiosData[index.row()][GenreDataType::key_type::TitleRole];
159​ break;
152 case ElisaUtils::Lyricist: 160​ case ElisaUtils::Lyricist:
153 case ElisaUtils::Composer: 161​ case ElisaUtils::Composer:
154 case ElisaUtils::FileName: 162​ case ElisaUtils::FileName:
155 case ElisaUtils::Unknown: 163​ case ElisaUtils::Unknown:
156 break; 164​ break;
157 } 165​ }
158 break; 166​ break;
159 case DatabaseInterface::ColumnsRoles::DurationRole: 167​ case DatabaseInterface::ColumnsRoles::DurationRole:
160 { 168​ {
161 if (d->mModelType == ElisaUtils::Track) { 169​ switch (d->mModelType)
170​ {
171​ case ElisaUtils::Track:
172​ {
162 auto trackDuration = d->mAllTrackData[index.row()][TrackDataType::key_type::DurationRole].toTime(); 173​ auto trackDuration = d->mAllTrackData[index.row()][TrackDataType::key_type::DurationRole].toTime();
163 if (trackDuration.hour() == 0) { 174​ if (trackDuration.hour() == 0) {
164 result = trackDuration.toString(QStringLiteral("mm:ss")); 175​ result = trackDuration.toString(QStringLiteral("mm:ss"));
165 } else { 176​ } else {
166 result = trackDuration.toString(); 177​ result = trackDuration.toString();
167 } 178​ }
179​ break;
180​ }
181​ case ElisaUtils::Radio:
182​ result = QStringLiteral("");
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…
183​ break;
184​ case ElisaUtils::Album:
185​ case ElisaUtils::Artist:
186​ case ElisaUtils::Genre:
187​ case ElisaUtils::Lyricist:
188​ case ElisaUtils::Composer:
189​ case ElisaUtils::FileName:
190​ case ElisaUtils::Unknown:
191​ break;
192​ }
193​ break;
194​ }
195​ case DatabaseInterface::ColumnsRoles::IsSingleDiscAlbumRole:
196​ {
197​ switch (d->mModelType)
198​ {
199​ case ElisaUtils::Track:
200​ case ElisaUtils::Radio:
201​ result = false;
202​ break;
203​ case ElisaUtils::Album:
204​ case ElisaUtils::Artist:
205​ case ElisaUtils::Genre:
206​ case ElisaUtils::Lyricist:
207​ case ElisaUtils::Composer:
208​ case ElisaUtils::FileName:
209​ case ElisaUtils::Unknown:
210​ break;
168 } 211​ }
169 break; 212​ break;
170 } 213​ }
171 default: 214​ default:
172 switch(d->mModelType) 215​ switch(d->mModelType)
173 { 216​ {
174 case ElisaUtils::Track: 217​ case ElisaUtils::Track:
175 result = d->mAllTrackData[index.row()][static_cast<TrackDataType::key_type>(role)]; 218​ result = d->mAllTrackData[index.row()][static_cast<TrackDataType::key_type>(role)];
176 break; 219​ break;
177 case ElisaUtils::Album: 220​ case ElisaUtils::Album:
178 result = d->mAllAlbumData[index.row()][static_cast<AlbumDataType::key_type>(role)]; 221​ result = d->mAllAlbumData[index.row()][static_cast<AlbumDataType::key_type>(role)];
179 break; 222​ break;
180 case ElisaUtils::Artist: 223​ case ElisaUtils::Artist:
181 result = d->mAllArtistData[index.row()][static_cast<ArtistDataType::key_type>(role)]; 224​ result = d->mAllArtistData[index.row()][static_cast<ArtistDataType::key_type>(role)];
182 break; 225​ break;
183 case ElisaUtils::Genre: 226​ case ElisaUtils::Genre:
184 result = d->mAllGenreData[index.row()][static_cast<GenreDataType::key_type>(role)]; 227​ result = d->mAllGenreData[index.row()][static_cast<GenreDataType::key_type>(role)];
185 break; 228​ break;
229​ case ElisaUtils::Radio:
230​ result = d->mAllRadiosData[index.row()][static_cast<TrackDataType::key_type>(role)];
231​ break;
186 case ElisaUtils::Lyricist: 232​ case ElisaUtils::Lyricist:
187 case ElisaUtils::Composer: 233​ case ElisaUtils::Composer:
188 case ElisaUtils::FileName: 234​ case ElisaUtils::FileName:
189 case ElisaUtils::Unknown: 235​ case ElisaUtils::Unknown:
190 break; 236​ break;
191 } 237​ }
192 } 238​ }
193 239​
▲ Show 20 Lines • Show All 159 Lines • ▼ Show 20 Line(s)396 for (result = 0; result < d->mAllTrackData.size(); ++result) {
353 } 399​ }
354 } 400​ }
355 401​
356 result = -1; 402​ result = -1;
357 403​
358 return result; 404​ return result;
359} 405​}
360 406​
407​int DataModel::radioIndexFromId(qulonglong id) const
408​{
409​ int result;
410​
411​ for (result = 0; result < d->mAllRadiosData.size(); ++result) {
412​ if (d->mAllRadiosData[result].databaseId() == id) {
413​ return result;
414​ }
415​ }
416​
417​ result = -1;
418​
419​ return result;
420​}
421​
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…
361void DataModel::connectModel(DatabaseInterface *database) 422​void DataModel::connectModel(DatabaseInterface *database)
362{ 423​{
363 d->mDataLoader.setDatabase(database); 424​ d->mDataLoader.setDatabase(database);
364 425​
365 connect(&d->mDataLoader, &ModelDataLoader::allTracksData, 426​ connect(&d->mDataLoader, &ModelDataLoader::allTracksData,
366 this, &DataModel::tracksAdded); 427​ this, &DataModel::tracksAdded);
428​ connect(&d->mDataLoader, &ModelDataLoader::allRadiosData,
429​ this, &DataModel::radiosAdded);
430​ connect(&d->mDataLoader, &ModelDataLoader::allRadiosDataUIUpdate,
431​ this, &DataModel::radiosModifiedUI);
367 connect(&d->mDataLoader, &ModelDataLoader::allAlbumsData, 432​ connect(&d->mDataLoader, &ModelDataLoader::allAlbumsData,
368 this, &DataModel::albumsAdded); 433​ this, &DataModel::albumsAdded);
369 connect(&d->mDataLoader, &ModelDataLoader::allArtistsData, 434​ connect(&d->mDataLoader, &ModelDataLoader::allArtistsData,
370 this, &DataModel::artistsAdded); 435​ this, &DataModel::artistsAdded);
371 connect(&d->mDataLoader, &ModelDataLoader::allGenresData, 436​ connect(&d->mDataLoader, &ModelDataLoader::allGenresData,
372 this, &DataModel::genresAdded); 437​ this, &DataModel::genresAdded);
373 connect(&d->mDataLoader, &ModelDataLoader::genresAdded, 438​ connect(&d->mDataLoader, &ModelDataLoader::genresAdded,
374 this, &DataModel::genresAdded); 439​ this, &DataModel::genresAdded);
375 connect(&d->mDataLoader, &ModelDataLoader::albumsAdded, 440​ connect(&d->mDataLoader, &ModelDataLoader::albumsAdded,
376 this, &DataModel::albumsAdded); 441​ this, &DataModel::albumsAdded);
377 connect(&d->mDataLoader, &ModelDataLoader::albumModified, 442​ connect(&d->mDataLoader, &ModelDataLoader::albumModified,
378 this, &DataModel::albumModified); 443​ this, &DataModel::albumModified);
379 connect(&d->mDataLoader, &ModelDataLoader::albumRemoved, 444​ connect(&d->mDataLoader, &ModelDataLoader::albumRemoved,
380 this, &DataModel::albumRemoved); 445​ this, &DataModel::albumRemoved);
381 connect(&d->mDataLoader, &ModelDataLoader::tracksAdded, 446​ connect(&d->mDataLoader, &ModelDataLoader::tracksAdded,
382 this, &DataModel::tracksAdded); 447​ this, &DataModel::tracksAdded);
383 connect(&d->mDataLoader, &ModelDataLoader::trackModified, 448​ connect(&d->mDataLoader, &ModelDataLoader::trackModified,
384 this, &DataModel::trackModified); 449​ this, &DataModel::trackModified);
385 connect(&d->mDataLoader, &ModelDataLoader::trackRemoved, 450​ connect(&d->mDataLoader, &ModelDataLoader::trackRemoved,
386 this, &DataModel::trackRemoved); 451​ this, &DataModel::trackRemoved);
387 connect(&d->mDataLoader, &ModelDataLoader::artistsAdded, 452​ connect(&d->mDataLoader, &ModelDataLoader::artistsAdded,
388 this, &DataModel::artistsAdded); 453​ this, &DataModel::artistsAdded);
389 connect(&d->mDataLoader, &ModelDataLoader::artistRemoved, 454​ connect(&d->mDataLoader, &ModelDataLoader::artistRemoved,
390 this, &DataModel::artistRemoved); 455​ this, &DataModel::artistRemoved);
456​ connect(&d->mDataLoader, &ModelDataLoader::radioRemoved,
457​ this, &DataModel::radioRemoved);
391} 458​}
392 459​
393void DataModel::tracksAdded(ListTrackDataType newData) 460​void DataModel::tracksAdded(ListTrackDataType newData)
394{ 461​{
395 if (newData.isEmpty() && d->mModelType == ElisaUtils::Track) { 462​ if (newData.isEmpty() && d->mModelType == ElisaUtils::Track) {
396 setBusy(false); 463​ setBusy(false);
397 } 464​ }
398 465​
▲ Show 20 Lines • Show All 47 Lines • ▼ Show 20 Line(s)506 } else {
446 } else { 513​ } else {
447 beginInsertRows({}, d->mAllTrackData.size(), d->mAllTrackData.size() + newData.size() - 1); 514​ beginInsertRows({}, d->mAllTrackData.size(), d->mAllTrackData.size() + newData.size() - 1);
448 d->mAllTrackData.append(newData); 515​ d->mAllTrackData.append(newData);
449 endInsertRows(); 516​ endInsertRows();
450 } 517​ }
451 } 518​ }
452} 519​}
453 520​
521​void DataModel::radiosAdded(ListTrackDataType newData)
522​{
523​ if (newData.isEmpty() && d->mModelType == ElisaUtils::Radio) {
524​ setBusy(false);
525​ }
526​
527​ if (newData.isEmpty() || d->mModelType != ElisaUtils::Radio) {
528​ return;
529​ }
530​
531​ if (d->mFilterType == ElisaUtils::FilterById && !d->mAllRadiosData.isEmpty()) {
532​ for (const auto &newTrack : newData) {
533​ auto trackIndex = trackIndexFromId(newTrack.databaseId());
534​
535​ if (trackIndex != -1) {
536​ continue;
537​ }
538​
539​ bool trackInserted = false;
540​ for (int trackIndex = 0; trackIndex < d->mAllRadiosData.count(); ++trackIndex) {
541​ const auto &oneTrack = d->mAllRadiosData[trackIndex];
542​
543​ if (oneTrack.trackNumber() > newTrack.trackNumber()) {
544​ beginInsertRows({}, trackIndex, trackIndex);
545​ d->mAllRadiosData.insert(trackIndex, newTrack);
546​ endInsertRows();
547​
548​ if (d->mAllRadiosData.size() == 1) {
549​ setBusy(false);
550​ }
551​
552​ trackInserted = true;
553​ break;
554​ }
555​ }
556​
557​ if (!trackInserted) {
558​ beginInsertRows({}, d->mAllRadiosData.count(), d->mAllRadiosData.count());
559​ d->mAllRadiosData.insert(d->mAllRadiosData.count(), newTrack);
560​ endInsertRows();
561​
562​ if (d->mAllRadiosData.size() == 1) {
563​ setBusy(false);
564​ }
565​ }
566​ }
567​ } else {
568​ if (d->mAllRadiosData.isEmpty()) {
569​ beginInsertRows({}, 0, newData.size() - 1);
570​ d->mAllRadiosData.swap(newData);
571​ endInsertRows();
572​
573​ setBusy(false);
574​ } else {
575​ beginInsertRows({}, d->mAllRadiosData.size(), d->mAllRadiosData.size() + newData.size() - 1);
576​ d->mAllRadiosData.append(newData);
577​ endInsertRows();
578​ }
579​ }
580​}
581​
454void DataModel::trackModified(const TrackDataType &modifiedTrack) 582​void DataModel::trackModified(const TrackDataType &modifiedTrack)
455{ 583​{
456 if (d->mModelType != ElisaUtils::Track) { 584​ if (d->mModelType != ElisaUtils::Track) {
457 return; 585​ return;
458 } 586​ }
459 587​
460 if (!d->mAlbumTitle.isEmpty() && !d->mAlbumArtist.isEmpty()) { 588​ if (!d->mAlbumTitle.isEmpty() && !d->mAlbumArtist.isEmpty()) {
461 if (modifiedTrack.album() != d->mAlbumTitle) { 589​ if (modifiedTrack.album() != d->mAlbumTitle) {
Show All 21 Lines601 } else {
483 auto position = itTrack - d->mAllTrackData.begin(); 611​ auto position = itTrack - d->mAllTrackData.begin();
484 612​
485 d->mAllTrackData[position] = modifiedTrack; 613​ d->mAllTrackData[position] = modifiedTrack;
486 614​
487 Q_EMIT dataChanged(index(position, 0), index(position, 0)); 615​ Q_EMIT dataChanged(index(position, 0), index(position, 0));
488 } 616​ }
489} 617​}
490 618​
619​void DataModel::radioModified(const TrackDataType &modifiedRadio)
620​{
621​ if (d->mModelType != ElisaUtils::Radio) {
622​ return;
623​ }
624​
625​ auto trackIndex = radioIndexFromId(modifiedRadio.databaseId());
626​
627​ if (trackIndex == -1) {
628​ return;
629​ }
630​
631​ d->mAllRadiosData[trackIndex] = modifiedRadio;
632​ Q_EMIT dataChanged(index(trackIndex, 0), index(trackIndex, 0));
633​}
634​
491void DataModel::trackRemoved(qulonglong removedTrackId) 635​void DataModel::trackRemoved(qulonglong removedTrackId)
492{ 636​{
493 if (d->mModelType != ElisaUtils::Track) { 637​ if (d->mModelType != ElisaUtils::Track) {
494 return; 638​ return;
495 } 639​ }
496 640​
497 if (!d->mAlbumTitle.isEmpty() && !d->mAlbumArtist.isEmpty()) { 641​ if (!d->mAlbumTitle.isEmpty() && !d->mAlbumArtist.isEmpty()) {
498 auto trackIndex = trackIndexFromId(removedTrackId); 642​ auto trackIndex = trackIndexFromId(removedTrackId);
Show All 16 Lines651 } else {
515 auto position = itTrack - d->mAllTrackData.begin(); 659​ auto position = itTrack - d->mAllTrackData.begin();
516 660​
517 beginRemoveRows({}, position, position); 661​ beginRemoveRows({}, position, position);
518 d->mAllTrackData.erase(itTrack); 662​ d->mAllTrackData.erase(itTrack);
519 endRemoveRows(); 663​ endRemoveRows();
520 } 664​ }
521} 665​}
522 666​
667​void DataModel::radioRemoved(qulonglong removedRadioId)
668​{
669​ if (d->mModelType != ElisaUtils::Radio) {
670​ return;
671​ }
672​
673​
674​ auto itRadio = std::find_if(d->mAllRadiosData.begin(), d->mAllRadiosData.end(),
675​ [removedRadioId](auto track) {return track.databaseId() == removedRadioId;});
676​
677​ if (itRadio == d->mAllRadiosData.end()) {
678​ return;
679​ }
680​
681​ auto position = itRadio - d->mAllRadiosData.begin();
682​
683​ beginRemoveRows({}, position, position);
684​ d->mAllRadiosData.erase(itRadio);
685​ endRemoveRows();
686​}
687​
688​void DataModel::radiosModifiedUI(const DataModel::TrackDataType radioData, bool isInsertion){
689​ if (d->mModelType != ElisaUtils::Radio) {
690​ return;
691​ }
692​
693​ if(isInsertion){
694​ ListTrackDataType list;
695​ list.append(radioData);
696​ radiosAdded(list);
697​ }else{
698​ radioModified(radioData);
699​ }
700​}
701​
702​void DataModel::removeRadios()
703​{
704​ if (d->mModelType != ElisaUtils::Radio) {
705​ return;
706​ }
707​
708​ beginRemoveRows({}, 0, d->mAllRadiosData.size());
709​ d->mAllRadiosData.clear();
710​ endRemoveRows();
711​}
712​
523void DataModel::genresAdded(DataModel::ListGenreDataType newData) 713​void DataModel::genresAdded(DataModel::ListGenreDataType newData)
524{ 714​{
525 if (newData.isEmpty() && d->mModelType == ElisaUtils::Genre) { 715​ if (newData.isEmpty() && d->mModelType == ElisaUtils::Genre) {
526 setBusy(false); 716​ setBusy(false);
527 } 717​ }
528 718​
529 if (newData.isEmpty() || d->mModelType != ElisaUtils::Genre) { 719​ if (newData.isEmpty() || d->mModelType != ElisaUtils::Genre) {
530 return; 720​ return;
▲ Show 20 Lines • Show All 140 Lines • Show Last 20 Lines