Codereview comments from r701
This commit is contained in:
parent
ed152be391
commit
2a33954725
@ -31,6 +31,7 @@
|
||||
#include <QLibrary>
|
||||
#include <QLibraryInfo>
|
||||
|
||||
using boost::shared_ptr;
|
||||
|
||||
const char* LibraryBackend::kDatabaseName = "clementine.db";
|
||||
const int LibraryBackend::kSchemaVersion = 7;
|
||||
@ -900,7 +901,7 @@ PlaylistItemList LibraryBackend::GetPlaylistItems(int playlist) {
|
||||
// The song table gets joined first, plus one for the song ROWID
|
||||
const int row = Song::kColumns.count() + 1;
|
||||
|
||||
boost::shared_ptr<PlaylistItem> item(
|
||||
shared_ptr<PlaylistItem> item(
|
||||
PlaylistItem::NewFromType(q.value(row + 0).toString()));
|
||||
if (!item)
|
||||
continue;
|
||||
@ -936,7 +937,7 @@ void LibraryBackend::SavePlaylist(int playlist, const PlaylistItemList& items) {
|
||||
return;
|
||||
|
||||
// Save the new ones
|
||||
foreach (boost::shared_ptr<PlaylistItem> item, items) {
|
||||
foreach (shared_ptr<PlaylistItem> item, items) {
|
||||
insert.bindValue(0, playlist);
|
||||
item->BindToQuery(&insert);
|
||||
|
||||
|
@ -60,6 +60,8 @@
|
||||
|
||||
#include <cmath>
|
||||
|
||||
using boost::shared_ptr;
|
||||
|
||||
#ifdef Q_OS_DARWIN
|
||||
// Non exported mac-specific function.
|
||||
void qt_mac_set_dock_menu(QMenu*);
|
||||
@ -223,10 +225,10 @@ MainWindow::MainWindow(QNetworkAccessManager* network, QWidget *parent)
|
||||
connect(library_, SIGNAL(TotalSongCountUpdated(int)), ui_.library_view, SLOT(TotalSongCountUpdated(int)));
|
||||
connect(library_, SIGNAL(ScanStarted()), SLOT(LibraryScanStarted()));
|
||||
connect(library_, SIGNAL(ScanFinished()), SLOT(LibraryScanFinished()));
|
||||
connect(library_, SIGNAL(BackendReady(boost::shared_ptr<LibraryBackendInterface>)),
|
||||
cover_manager_.get(), SLOT(SetBackend(boost::shared_ptr<LibraryBackendInterface>)));
|
||||
connect(library_, SIGNAL(BackendReady(boost::shared_ptr<LibraryBackendInterface>)),
|
||||
playlist_, SLOT(SetBackend(boost::shared_ptr<LibraryBackendInterface>)));
|
||||
connect(library_, SIGNAL(BackendReady(shared_ptr<LibraryBackendInterface>)),
|
||||
cover_manager_.get(), SLOT(SetBackend(shared_ptr<LibraryBackendInterface>)));
|
||||
connect(library_, SIGNAL(BackendReady(shared_ptr<LibraryBackendInterface>)),
|
||||
playlist_, SLOT(SetBackend(shared_ptr<LibraryBackendInterface>)));
|
||||
|
||||
// Age filters
|
||||
QActionGroup* filter_age_group = new QActionGroup(this);
|
||||
|
@ -30,6 +30,8 @@
|
||||
|
||||
#include <boost/bind.hpp>
|
||||
|
||||
using boost::shared_ptr;
|
||||
|
||||
#ifdef Q_WS_X11
|
||||
QDBusArgument& operator<< (QDBusArgument& arg, const DBusStatus& status) {
|
||||
arg.beginStructure();
|
||||
@ -210,7 +212,7 @@ void Player::PlayAt(int index, Engine::TrackChangeType change) {
|
||||
playlist_->set_current_index(-1); // to reshuffle
|
||||
playlist_->set_current_index(index);
|
||||
|
||||
boost::shared_ptr<PlaylistItem> item = playlist_->item_at(index);
|
||||
shared_ptr<PlaylistItem> item = playlist_->item_at(index);
|
||||
current_item_options_ = item->options();
|
||||
current_item_ = item->Metadata();
|
||||
|
||||
@ -231,7 +233,7 @@ void Player::StreamReady(const QUrl& original_url, const QUrl& media_url) {
|
||||
if (current_index == -1)
|
||||
return;
|
||||
|
||||
boost::shared_ptr<PlaylistItem> item = playlist_->item_at(current_index);
|
||||
shared_ptr<PlaylistItem> item = playlist_->item_at(current_index);
|
||||
if (!item || item->Url() != original_url)
|
||||
return;
|
||||
|
||||
@ -256,7 +258,7 @@ void Player::Seek(int seconds) {
|
||||
}
|
||||
|
||||
void Player::EngineMetadataReceived(const Engine::SimpleMetaBundle& bundle) {
|
||||
boost::shared_ptr<PlaylistItem> item = playlist_->current_item();
|
||||
shared_ptr<PlaylistItem> item = playlist_->current_item();
|
||||
if (item == NULL)
|
||||
return;
|
||||
|
||||
@ -344,7 +346,7 @@ QVariantMap Player::GetMetadata(const PlaylistItem& item) const {
|
||||
}
|
||||
|
||||
QVariantMap Player::GetMetadata() const {
|
||||
boost::shared_ptr<PlaylistItem> item = playlist_->current_item();
|
||||
shared_ptr<PlaylistItem> item = playlist_->current_item();
|
||||
if (item) {
|
||||
return GetMetadata(*item);
|
||||
}
|
||||
|
@ -36,6 +36,8 @@
|
||||
|
||||
#include <lastfm/ScrobblePoint>
|
||||
|
||||
using boost::shared_ptr;
|
||||
|
||||
const char* Playlist::kRowsMimetype = "application/x-clementine-playlist-rows";
|
||||
const char* Playlist::kSettingsGroup = "Playlist";
|
||||
|
||||
@ -143,7 +145,7 @@ QVariant Playlist::data(const QModelIndex& index, int role) const {
|
||||
case Qt::EditRole:
|
||||
case Qt::ToolTipRole:
|
||||
case Qt::DisplayRole: {
|
||||
const boost::shared_ptr<PlaylistItem>& item = items_[index.row()];
|
||||
shared_ptr<PlaylistItem> item = items_[index.row()];
|
||||
Song song = item->Metadata();
|
||||
|
||||
// Don't forget to change Playlist::CompareItems when adding new columns
|
||||
@ -462,7 +464,7 @@ QModelIndex Playlist::InsertItems(const PlaylistItemList& items, int after) {
|
||||
QModelIndex Playlist::InsertLibraryItems(const SongList& songs, int after) {
|
||||
PlaylistItemList items;
|
||||
foreach (const Song& song, songs) {
|
||||
items << boost::shared_ptr<PlaylistItem>(new LibraryPlaylistItem(song));
|
||||
items << shared_ptr<PlaylistItem>(new LibraryPlaylistItem(song));
|
||||
}
|
||||
return InsertItems(items, after);
|
||||
}
|
||||
@ -470,7 +472,7 @@ QModelIndex Playlist::InsertLibraryItems(const SongList& songs, int after) {
|
||||
QModelIndex Playlist::InsertSongs(const SongList& songs, int after) {
|
||||
PlaylistItemList items;
|
||||
foreach (const Song& song, songs) {
|
||||
items << boost::shared_ptr<PlaylistItem>(new SongPlaylistItem(song));
|
||||
items << shared_ptr<PlaylistItem>(new SongPlaylistItem(song));
|
||||
}
|
||||
return InsertItems(items, after);
|
||||
}
|
||||
@ -481,7 +483,7 @@ QModelIndex Playlist::InsertRadioStations(const QList<RadioItem*>& items, int af
|
||||
if (!item->playable)
|
||||
continue;
|
||||
|
||||
playlist_items << boost::shared_ptr<PlaylistItem>(
|
||||
playlist_items << shared_ptr<PlaylistItem>(
|
||||
new RadioPlaylistItem(item->service, item->Url(), item->Title(), item->Artist()));
|
||||
}
|
||||
return InsertItems(playlist_items, after);
|
||||
@ -490,7 +492,7 @@ QModelIndex Playlist::InsertRadioStations(const QList<RadioItem*>& items, int af
|
||||
QModelIndex Playlist::InsertStreamUrls(const QList<QUrl>& urls, int after) {
|
||||
PlaylistItemList playlist_items;
|
||||
foreach (const QUrl& url, urls) {
|
||||
playlist_items << boost::shared_ptr<PlaylistItem>(new RadioPlaylistItem(
|
||||
playlist_items << shared_ptr<PlaylistItem>(new RadioPlaylistItem(
|
||||
RadioModel::ServiceByName(SavedRadio::kServiceName), url.toString(), url.toString(), QString()));
|
||||
}
|
||||
return InsertItems(playlist_items, after);
|
||||
@ -522,10 +524,10 @@ QMimeData* Playlist::mimeData(const QModelIndexList& indexes) const {
|
||||
}
|
||||
|
||||
bool Playlist::CompareItems(int column, Qt::SortOrder order,
|
||||
boost::shared_ptr<PlaylistItem> _a,
|
||||
boost::shared_ptr<PlaylistItem> _b) {
|
||||
boost::shared_ptr<PlaylistItem> a = order == Qt::AscendingOrder ? _a : _b;
|
||||
boost::shared_ptr<PlaylistItem> b = order == Qt::AscendingOrder ? _b : _a;
|
||||
shared_ptr<PlaylistItem> _a,
|
||||
shared_ptr<PlaylistItem> _b) {
|
||||
shared_ptr<PlaylistItem> a = order == Qt::AscendingOrder ? _a : _b;
|
||||
shared_ptr<PlaylistItem> b = order == Qt::AscendingOrder ? _b : _a;
|
||||
|
||||
#define cmp(field) return a->Metadata().field() < b->Metadata().field()
|
||||
|
||||
@ -591,7 +593,7 @@ void Playlist::sort(int column, Qt::SortOrder order) {
|
||||
layoutAboutToBeChanged();
|
||||
|
||||
// This is a slow and nasty way to keep the persistent indices
|
||||
QMap<int, boost::shared_ptr<PlaylistItem> > old_persistent_mappings;
|
||||
QMap<int, shared_ptr<PlaylistItem> > old_persistent_mappings;
|
||||
foreach (const QModelIndex& index, persistentIndexList()) {
|
||||
old_persistent_mappings[index.row()] = items_[index.row()];
|
||||
}
|
||||
@ -599,7 +601,7 @@ void Playlist::sort(int column, Qt::SortOrder order) {
|
||||
qStableSort(items_.begin(), items_.end(),
|
||||
boost::bind(&Playlist::CompareItems, column, order, _1, _2));
|
||||
|
||||
QMapIterator<int, boost::shared_ptr<PlaylistItem> > it(old_persistent_mappings);
|
||||
QMapIterator<int, shared_ptr<PlaylistItem> > it(old_persistent_mappings);
|
||||
while (it.hasNext()) {
|
||||
it.next();
|
||||
for (int col=0 ; col<ColumnCount ; ++col) {
|
||||
@ -637,7 +639,7 @@ void Playlist::SetCurrentIsPaused(bool paused) {
|
||||
index(current_item_.row(), ColumnCount));
|
||||
}
|
||||
|
||||
void Playlist::SetBackend(boost::shared_ptr<LibraryBackendInterface> backend) {
|
||||
void Playlist::SetBackend(shared_ptr<LibraryBackendInterface> backend) {
|
||||
backend_ = backend;
|
||||
|
||||
Restore();
|
||||
@ -720,7 +722,7 @@ void Playlist::SetStreamMetadata(const QUrl& url, const Song& song) {
|
||||
if (!current_item_.isValid())
|
||||
return;
|
||||
|
||||
boost::shared_ptr<PlaylistItem> item = items_[current_item_.row()];
|
||||
shared_ptr<PlaylistItem> item = items_[current_item_.row()];
|
||||
if (item->Url() != url)
|
||||
return;
|
||||
|
||||
@ -740,7 +742,7 @@ void Playlist::ClearStreamMetadata() {
|
||||
if (!current_item_.isValid())
|
||||
return;
|
||||
|
||||
boost::shared_ptr<PlaylistItem> item = items_[current_item_.row()];
|
||||
shared_ptr<PlaylistItem> item = items_[current_item_.row()];
|
||||
item->ClearTemporaryMetadata();
|
||||
UpdateScrobblePoint();
|
||||
|
||||
@ -752,16 +754,16 @@ bool Playlist::stop_after_current() const {
|
||||
stop_after_.row() == current_item_.row();
|
||||
}
|
||||
|
||||
boost::shared_ptr<PlaylistItem> Playlist::current_item() const {
|
||||
shared_ptr<PlaylistItem> Playlist::current_item() const {
|
||||
int i = current_index();
|
||||
if (i == -1)
|
||||
return boost::shared_ptr<PlaylistItem>();
|
||||
return shared_ptr<PlaylistItem>();
|
||||
|
||||
return item_at(i);
|
||||
}
|
||||
|
||||
PlaylistItem::Options Playlist::current_item_options() const {
|
||||
boost::shared_ptr<PlaylistItem> item = current_item();
|
||||
shared_ptr<PlaylistItem> item = current_item();
|
||||
if (!item)
|
||||
return PlaylistItem::Default;
|
||||
|
||||
@ -769,7 +771,7 @@ PlaylistItem::Options Playlist::current_item_options() const {
|
||||
}
|
||||
|
||||
Song Playlist::current_item_metadata() const {
|
||||
boost::shared_ptr<PlaylistItem> item = current_item();
|
||||
shared_ptr<PlaylistItem> item = current_item();
|
||||
if (!item)
|
||||
return Song();
|
||||
|
||||
|
@ -23,6 +23,7 @@
|
||||
|
||||
#include <QtDebug>
|
||||
|
||||
using boost::shared_ptr;
|
||||
using ::testing::Return;
|
||||
|
||||
namespace {
|
||||
@ -53,10 +54,10 @@ class PlaylistTest : public ::testing::Test {
|
||||
return ret;
|
||||
}
|
||||
|
||||
boost::shared_ptr<PlaylistItem> MakeMockItemP(
|
||||
shared_ptr<PlaylistItem> MakeMockItemP(
|
||||
const QString& title, const QString& artist = QString(),
|
||||
const QString& album = QString(), int length = 123) const {
|
||||
return boost::shared_ptr<PlaylistItem>(MakeMockItem(title, artist, album, length));
|
||||
return shared_ptr<PlaylistItem>(MakeMockItem(title, artist, album, length));
|
||||
}
|
||||
|
||||
Playlist playlist_;
|
||||
@ -69,7 +70,7 @@ TEST_F(PlaylistTest, Basic) {
|
||||
|
||||
TEST_F(PlaylistTest, InsertItems) {
|
||||
MockPlaylistItem* item = MakeMockItem("Title", "Artist", "Album", 123);
|
||||
boost::shared_ptr<PlaylistItem> item_ptr(item);
|
||||
shared_ptr<PlaylistItem> item_ptr(item);
|
||||
|
||||
// Insert the item
|
||||
EXPECT_EQ(0, playlist_.rowCount(QModelIndex()));
|
||||
|
Loading…
x
Reference in New Issue
Block a user