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