From b7386c4fa6c9d9f383639f5f418be618f7c38bba Mon Sep 17 00:00:00 2001 From: Andrzej Rybczak Date: Thu, 8 Dec 2016 04:28:43 +0100 Subject: [PATCH] song list: get rid of boost::zip_iterator and improve {Const,}SongIterator --- src/Makefile.am | 1 + src/actions.cpp | 44 ++++++------ src/actions.h | 8 +-- src/browser.cpp | 41 ++++++----- src/display.cpp | 3 +- src/helpers.cpp | 2 +- src/helpers.h | 6 +- src/helpers/song_iterator_maker.h | 48 +++++++++---- src/menu.h | 11 ++- src/mpdpp.h | 17 +++++ src/search_engine.cpp | 44 ++++++------ src/song_list.cpp | 31 ++------ src/song_list.h | 84 ++++++++++++++++++---- src/tag_editor.cpp | 27 ++----- .../song_iterator_maker.h => utility/const.h} | 29 ++------ 15 files changed, 216 insertions(+), 180 deletions(-) copy src/{helpers/song_iterator_maker.h => utility/const.h} (60%) diff --git a/src/Makefile.am b/src/Makefile.am index 46a3fbe..397b671 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -59,6 +59,7 @@ ncmpcpp_LDFLAGS = $(all_libraries) noinst_HEADERS = \ helpers/song_iterator_maker.h \ utility/comparators.h \ + utility/const.h \ utility/conversion.h \ utility/functional.h \ utility/html.h \ diff --git a/src/actions.cpp b/src/actions.cpp index 3b4bf88..03a7bad 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -80,9 +80,9 @@ std::vector> AvailableActions; void populateActions(); -bool scrollTagCanBeRun(NC::List *&list, SongList *&songs); -void scrollTagUpRun(NC::List *list, SongList *songs, MPD::Song::GetFunction get); -void scrollTagDownRun(NC::List *list, SongList *songs, MPD::Song::GetFunction get); +bool scrollTagCanBeRun(NC::List *&list, const SongList *&songs); +void scrollTagUpRun(NC::List *list, const SongList *songs, MPD::Song::GetFunction get); +void scrollTagDownRun(NC::List *list, const SongList *songs, MPD::Song::GetFunction get); void seek(); void findItem(const SearchDirection direction); @@ -1783,28 +1783,25 @@ bool SelectAlbum::canBeRun() void SelectAlbum::run() { const auto front = m_songs->beginS(), current = m_songs->currentS(), end = m_songs->endS(); - auto *s = current->get(); - if (s == nullptr) + if (current->song() == nullptr) return; auto get = &MPD::Song::getAlbum; - const std::string tag = s->getTags(get); + const std::string tag = current->song()->getTags(get); // go up for (auto it = current; it != front;) { --it; - s = it->get(); - if (s == nullptr || s->getTags(get) != tag) + if (it->song() == nullptr || it->song()->getTags(get) != tag) break; - it->get().setSelected(true); + it->properties().setSelected(true); } // go down for (auto it = current;;) { - it->get().setSelected(true); + it->properties().setSelected(true); if (++it == end) break; - s = it->get(); - if (s == nullptr || s->getTags(get) != tag) + if (it->song() == nullptr || it->song()->getTags(get) != tag) break; } Statusbar::print("Album around cursor position selected"); @@ -2809,7 +2806,7 @@ void populateActions() } } -bool scrollTagCanBeRun(NC::List *&list, SongList *&songs) +bool scrollTagCanBeRun(NC::List *&list, const SongList *&songs) { auto w = myScreen->activeWindow(); if (list != static_cast(w)) @@ -2820,36 +2817,34 @@ bool scrollTagCanBeRun(NC::List *&list, SongList *&songs) && songs != nullptr; } -void scrollTagUpRun(NC::List *list, SongList *songs, MPD::Song::GetFunction get) +void scrollTagUpRun(NC::List *list, const SongList *songs, MPD::Song::GetFunction get) { const auto front = songs->beginS(); auto it = songs->currentS(); - if (auto *s = it->get()) + if (it->song() != nullptr) { - const std::string tag = s->getTags(get); + const std::string tag = it->song()->getTags(get); while (it != front) { --it; - s = it->get(); - if (s == nullptr || s->getTags(get) != tag) + if (it->song() == nullptr || it->song()->getTags(get) != tag) break; } list->highlight(it-front); } } -void scrollTagDownRun(NC::List *list, SongList *songs, MPD::Song::GetFunction get) +void scrollTagDownRun(NC::List *list, const SongList *songs, MPD::Song::GetFunction get) { const auto front = songs->beginS(), back = --songs->endS(); auto it = songs->currentS(); - if (auto *s = it->get()) + if (it->song() != nullptr) { - const std::string tag = s->getTags(get); + const std::string tag = it->song()->getTags(get); while (it != back) { ++it; - s = it->get(); - if (s == nullptr || s->getTags(get) != tag) + if (it->song() == nullptr || it->song()->getTags(get) != tag) break; } list->highlight(it-front); @@ -2882,7 +2877,8 @@ void seek() // can be run and one of them is of the given type. This will still not work // in some contrived cases, but allows for more flexibility than accepting // single actions only. - auto hasRunnableAction = [](BindingsConfiguration::BindingIteratorPair &bindings, Actions::Type type) { + auto hasRunnableAction = [](BindingsConfiguration::BindingIteratorPair &bindings, + Actions::Type type) { bool success = false; for (auto binding = bindings.first; binding != bindings.second; ++binding) { diff --git a/src/actions.h b/src/actions.h index 7806acb..1254cf6 100644 --- a/src/actions.h +++ b/src/actions.h @@ -280,7 +280,7 @@ private: virtual void run() override; NC::List *m_list; - SongList *m_songs; + const SongList *m_songs; }; struct ScrollUpAlbum: BaseAction @@ -292,7 +292,7 @@ private: virtual void run() override; NC::List *m_list; - SongList *m_songs; + const SongList *m_songs; }; struct ScrollDownArtist: BaseAction @@ -304,7 +304,7 @@ private: virtual void run() override; NC::List *m_list; - SongList *m_songs; + const SongList *m_songs; }; struct ScrollDownAlbum: BaseAction @@ -316,7 +316,7 @@ private: virtual void run() override; NC::List *m_list; - SongList *m_songs; + const SongList *m_songs; }; struct PageUp: BaseAction diff --git a/src/browser.cpp b/src/browser.cpp index 7544a6c..dd1e733 100644 --- a/src/browser.cpp +++ b/src/browser.cpp @@ -70,54 +70,53 @@ void clearDirectory(const std::string &directory); std::string itemToString(const MPD::Item &item); bool browserEntryMatcher(const Regex::Regex &rx, const MPD::Item &item, bool filter); -template -struct SongExtractor -{ - typedef SongExtractor type; - - typedef typename NC::Menu::Item MenuItem; - typedef typename std::conditional::type Item; - typedef typename std::conditional::type Song; +} - Song *operator()(Item &item) const +template <> +struct SongPropertiesExtractor +{ + template + auto &operator()(ItemT &item) const { - Song *ptr = nullptr; - if (item.value().type() == MPD::Item::Type::Song) - ptr = const_cast(&item.value().song()); - return ptr; + auto s = item.value().type() == MPD::Item::Type::Song + ? &item.value().song() + : nullptr; + m_cache.assign(&item.properties(), s); + return m_cache; } -}; -} +private: + mutable SongProperties m_cache; +}; SongIterator BrowserWindow::currentS() { - return makeSongIterator_(current(), SongExtractor()); + return makeSongIterator(current()); } ConstSongIterator BrowserWindow::currentS() const { - return makeConstSongIterator_(current(), SongExtractor()); + return makeConstSongIterator(current()); } SongIterator BrowserWindow::beginS() { - return makeSongIterator_(begin(), SongExtractor()); + return makeSongIterator(begin()); } ConstSongIterator BrowserWindow::beginS() const { - return makeConstSongIterator_(begin(), SongExtractor()); + return makeConstSongIterator(begin()); } SongIterator BrowserWindow::endS() { - return makeSongIterator_(end(), SongExtractor()); + return makeSongIterator(end()); } ConstSongIterator BrowserWindow::endS() const { - return makeConstSongIterator_(end(), SongExtractor()); + return makeConstSongIterator(end()); } std::vector BrowserWindow::getSelectedSongs() diff --git a/src/display.cpp b/src/display.cpp index e50915b..8fdda0e 100644 --- a/src/display.cpp +++ b/src/display.cpp @@ -87,8 +87,7 @@ void setProperties(NC::Menu &menu, const MPD::Song &s, const SongList &list, auto next = list.beginS() + drawn_pos + 1; if (next != list.endS()) { - auto next_s = next->get(); - if (next_s != nullptr && next_s->getAlbum() != s.getAlbum()) + if (next->song() != nullptr && next->song()->getAlbum() != s.getAlbum()) separate_albums = true; } } diff --git a/src/helpers.cpp b/src/helpers.cpp index a53ed77..0465a7b 100644 --- a/src/helpers.cpp +++ b/src/helpers.cpp @@ -35,7 +35,7 @@ const MPD::Song *currentSong(const BaseScreen *screen) { const auto it = list->currentS(); if (it != list->endS()) - ptr = it->get(); + ptr = it->song(); } return ptr; } diff --git a/src/helpers.h b/src/helpers.h index e275504..b9eb76f 100644 --- a/src/helpers.h +++ b/src/helpers.h @@ -387,12 +387,10 @@ template void markSongsInPlaylist(ListT &list) { ScopedUnfilteredMenu sunfilter(ReapplyFilter::No, list); - MPD::Song *s; for (auto &p : static_cast(list)) { - s = p.get(); - if (s != nullptr) - p.get().setBold(myPlaylist->checkForSong(*s)); + if (p.song() != nullptr) + p.properties().setBold(myPlaylist->checkForSong(*p.song())); } } diff --git a/src/helpers/song_iterator_maker.h b/src/helpers/song_iterator_maker.h index 87e21dc..37d4f59 100644 --- a/src/helpers/song_iterator_maker.h +++ b/src/helpers/song_iterator_maker.h @@ -22,26 +22,48 @@ #define NCMPCPP_HELPERS_SONG_ITERATOR_MAKER_H #include -#include #include "menu.h" #include "song_list.h" -template -SongIterator makeSongIterator_(typename NC::Menu::Iterator it, TransformT &&map) +template +struct SongPropertiesExtractor { - return SongIterator(boost::make_zip_iterator(boost::make_tuple( - typename NC::Menu::PropertiesIterator(it), - boost::make_transform_iterator(it, std::forward(map)) - ))); + template + auto &operator()(ItemT &item) const + { + return m_cache.assign(&item.properties(), &item.value()); + } + +private: + mutable SongProperties m_cache; +}; + +template +SongIterator makeSongIterator(IteratorT it) +{ + typedef SongPropertiesExtractor< + typename IteratorT::value_type::Type + > Extractor; + static_assert( + std::is_convertible< + typename std::result_of::type, + SongProperties & + >::value, "invalid result type of SongPropertiesExtractor"); + return SongIterator(boost::make_transform_iterator(it, Extractor{})); } -template -ConstSongIterator makeConstSongIterator_(typename NC::Menu::ConstIterator it, TransformT &&map) +template +ConstSongIterator makeConstSongIterator(ConstIteratorT it) { - return ConstSongIterator(boost::make_zip_iterator(boost::make_tuple( - typename NC::Menu::ConstPropertiesIterator(it), - boost::make_transform_iterator(it, std::forward(map)) - ))); + typedef SongPropertiesExtractor< + typename ConstIteratorT::value_type::Type + > Extractor; + static_assert( + std::is_convertible< + typename std::result_of::type, + const SongProperties & + >::value, "invalid result type of SongPropertiesExtractor"); + return ConstSongIterator(boost::make_transform_iterator(it, Extractor{})); } #endif // NCMPCPP_HELPERS_SONG_ITERATOR_MAKER_H diff --git a/src/menu.h b/src/menu.h index da87763..1cd3aaa 100644 --- a/src/menu.h +++ b/src/menu.h @@ -29,6 +29,7 @@ #include #include +#include "utility/const.h" #include "strbuffer.h" #include "window.h" @@ -197,8 +198,6 @@ struct Menu: Window, List } private: - enum class Const { Yes, No }; - template struct ExtractProperties { @@ -254,19 +253,19 @@ struct Menu: Window, List typedef std::reverse_iterator ConstReverseIterator; typedef boost::transform_iterator< - typename Item::template ExtractValue, + typename Item::template ExtractValue, Iterator> ValueIterator; typedef boost::transform_iterator< - typename Item::template ExtractValue, + typename Item::template ExtractValue, ConstIterator> ConstValueIterator; typedef std::reverse_iterator ReverseValueIterator; typedef std::reverse_iterator ConstReverseValueIterator; typedef boost::transform_iterator< - typename Item::template ExtractProperties, + typename Item::template ExtractProperties, Iterator> PropertiesIterator; typedef boost::transform_iterator< - typename Item::template ExtractProperties, + typename Item::template ExtractProperties, ConstIterator> ConstPropertiesIterator; /// Function helper prototype used to display each option on the screen. diff --git a/src/mpdpp.h b/src/mpdpp.h index c0a1c64..0f85750 100644 --- a/src/mpdpp.h +++ b/src/mpdpp.h @@ -260,6 +260,23 @@ struct Item { return m_type; } + + Directory &directory() + { + return const_cast( + static_cast(*this).directory()); + } + Song &song() + { + return const_cast( + static_cast(*this).song()); + } + Playlist &playlist() + { + return const_cast( + static_cast(*this).playlist()); + } + const Directory &directory() const { assert(m_type == Type::Directory); diff --git a/src/search_engine.cpp b/src/search_engine.cpp index ee808ea..9bf5501 100644 --- a/src/search_engine.cpp +++ b/src/search_engine.cpp @@ -73,56 +73,56 @@ namespace pos { }*/ std::string SEItemToString(const SEItem &ei); -bool SEItemEntryMatcher(const Regex::Regex &rx, const NC::Menu::Item &item, bool filter); +bool SEItemEntryMatcher(const Regex::Regex &rx, + const NC::Menu::Item &item, + bool filter); -template -struct SongExtractor -{ - typedef SongExtractor type; - - typedef typename NC::Menu::Item MenuItem; - typedef typename std::conditional::type Item; - typedef typename std::conditional::type Song; +} - Song *operator()(Item &item) const +template <> +struct SongPropertiesExtractor +{ + template + auto &operator()(ItemT &item) const { - Song *ptr = nullptr; - if (!item.isSeparator() && item.value().isSong()) - ptr = &item.value().song(); - return ptr; + auto s = !item.isSeparator() && item.value().isSong() + ? &item.value().song() + : nullptr; + return m_cache.assign(&item.properties(), s); } -}; -} +private: + mutable SongProperties m_cache; +}; SongIterator SearchEngineWindow::currentS() { - return makeSongIterator_(current(), SongExtractor()); + return makeSongIterator(current()); } ConstSongIterator SearchEngineWindow::currentS() const { - return makeConstSongIterator_(current(), SongExtractor()); + return makeConstSongIterator(current()); } SongIterator SearchEngineWindow::beginS() { - return makeSongIterator_(begin(), SongExtractor()); + return makeSongIterator(begin()); } ConstSongIterator SearchEngineWindow::beginS() const { - return makeConstSongIterator_(begin(), SongExtractor()); + return makeConstSongIterator(begin()); } SongIterator SearchEngineWindow::endS() { - return makeSongIterator_(end(), SongExtractor()); + return makeSongIterator(end()); } ConstSongIterator SearchEngineWindow::endS() const { - return makeConstSongIterator_(end(), SongExtractor()); + return makeConstSongIterator(end()); } std::vector SearchEngineWindow::getSelectedSongs() diff --git a/src/song_list.cpp b/src/song_list.cpp index 5de4dc1..b1acd6f 100644 --- a/src/song_list.cpp +++ b/src/song_list.cpp @@ -22,53 +22,34 @@ #include "song_info.h" #include "utility/functional.h" -namespace { - -template -struct SongExtractor -{ - typedef SongExtractor type; - - typedef typename NC::Menu::Item MenuItem; - typedef typename std::conditional::type Item; - typedef typename std::conditional::type Song; - - Song *operator()(Item &item) const - { - return &item.value(); - } -}; - -} - SongIterator SongMenu::currentS() { - return makeSongIterator_(current(), SongExtractor()); + return makeSongIterator(current()); } ConstSongIterator SongMenu::currentS() const { - return makeConstSongIterator_(current(), SongExtractor()); + return makeConstSongIterator(current()); } SongIterator SongMenu::beginS() { - return makeSongIterator_(begin(), SongExtractor()); + return makeSongIterator(begin()); } ConstSongIterator SongMenu::beginS() const { - return makeConstSongIterator_(begin(), SongExtractor()); + return makeConstSongIterator(begin()); } SongIterator SongMenu::endS() { - return makeSongIterator_(end(), SongExtractor()); + return makeSongIterator(end()); } ConstSongIterator SongMenu::endS() const { - return makeConstSongIterator_(end(), SongExtractor()); + return makeConstSongIterator(end()); } std::vector SongMenu::getSelectedSongs() diff --git a/src/song_list.h b/src/song_list.h index 3b8a0b9..184c1dc 100644 --- a/src/song_list.h +++ b/src/song_list.h @@ -22,25 +22,85 @@ #define NCMPCPP_SONG_LIST_H #include -#include #include "menu.h" #include "song.h" +#include "utility/const.h" -template +struct SongProperties +{ + enum class State { Undefined, Const, Mutable }; + + SongProperties() + : m_state(State::Undefined) + { } + + SongProperties &assign(NC::List::Properties *properties_, MPD::Song *song_) + { + m_state = State::Mutable; + m_properties = properties_; + m_song = song_; + return *this; + } + + SongProperties &assign(const NC::List::Properties *properties_, const MPD::Song *song_) + { + m_state = State::Const; + m_const_properties = properties_; + m_const_song = song_; + return *this; + } + + const NC::List::Properties &properties() const + { + assert(m_state != State::Undefined); + return *m_const_properties; + } + const MPD::Song *song() const + { + assert(m_state != State::Undefined); + return m_const_song; + } + + NC::List::Properties &properties() + { + assert(m_state == State::Mutable); + return *m_properties; + } + MPD::Song *song() + { + assert(m_state == State::Mutable); + return m_song; + } + +private: + State m_state; + + union { + NC::List::Properties *m_properties; + const NC::List::Properties *m_const_properties; + }; + union { + MPD::Song *m_song; + const MPD::Song *m_const_song; + }; +}; + +template using SongIteratorT = boost::range_detail::any_iterator< - ValueT, + typename std::conditional< + const_ == Const::Yes, + const SongProperties, + SongProperties>::type, boost::random_access_traversal_tag, - const ValueT, // const needed, see https://svn.boost.org/trac/boost/ticket/10493 + typename std::conditional< + const_ == Const::Yes, + const SongProperties &, + SongProperties &>::type, std::ptrdiff_t ->; - -typedef SongIteratorT> SongIterator; -typedef SongIteratorT> ConstSongIterator; + >; -namespace Bit { -const size_t Properties = 0; -const size_t Song = 1; -} +typedef SongIteratorT SongIterator; +typedef SongIteratorT ConstSongIterator; struct SongList { diff --git a/src/tag_editor.cpp b/src/tag_editor.cpp index 3dcaac5..645f8c9 100644 --- a/src/tag_editor.cpp +++ b/src/tag_editor.cpp @@ -87,51 +87,36 @@ std::string SongToString(const MPD::MutableSong &s); bool DirEntryMatcher(const Regex::Regex &rx, const std::pair &dir, bool filter); bool SongEntryMatcher(const Regex::Regex &rx, const MPD::MutableSong &s); -template -struct SongExtractor -{ - typedef SongExtractor type; - - typedef typename NC::Menu::Item MenuItem; - typedef typename std::conditional::type Item; - typedef typename std::conditional::type Song; - - Song *operator()(Item &item) const - { - return &item.value(); - } -}; - } SongIterator TagsWindow::currentS() { - return makeSongIterator_(current(), SongExtractor()); + return makeSongIterator(current()); } ConstSongIterator TagsWindow::currentS() const { - return makeConstSongIterator_(current(), SongExtractor()); + return makeConstSongIterator(current()); } SongIterator TagsWindow::beginS() { - return makeSongIterator_(begin(), SongExtractor()); + return makeSongIterator(begin()); } ConstSongIterator TagsWindow::beginS() const { - return makeConstSongIterator_(begin(), SongExtractor()); + return makeConstSongIterator(begin()); } SongIterator TagsWindow::endS() { - return makeSongIterator_(end(), SongExtractor()); + return makeSongIterator(end()); } ConstSongIterator TagsWindow::endS() const { - return makeConstSongIterator_(end(), SongExtractor()); + return makeConstSongIterator(end()); } std::vector TagsWindow::getSelectedSongs() diff --git a/src/helpers/song_iterator_maker.h b/src/utility/const.h similarity index 60% copy from src/helpers/song_iterator_maker.h copy to src/utility/const.h index 87e21dc..232ab4a 100644 --- a/src/helpers/song_iterator_maker.h +++ b/src/utility/const.h @@ -18,30 +18,9 @@ * 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA. * ***************************************************************************/ -#ifndef NCMPCPP_HELPERS_SONG_ITERATOR_MAKER_H -#define NCMPCPP_HELPERS_SONG_ITERATOR_MAKER_H +#ifndef NCMPCPP_UTILITY_CONST_H +#define NCMPCPP_UTILITY_CONST_H -#include -#include -#include "menu.h" -#include "song_list.h" +enum class Const { Yes, No }; -template -SongIterator makeSongIterator_(typename NC::Menu::Iterator it, TransformT &&map) -{ - return SongIterator(boost::make_zip_iterator(boost::make_tuple( - typename NC::Menu::PropertiesIterator(it), - boost::make_transform_iterator(it, std::forward(map)) - ))); -} - -template -ConstSongIterator makeConstSongIterator_(typename NC::Menu::ConstIterator it, TransformT &&map) -{ - return ConstSongIterator(boost::make_zip_iterator(boost::make_tuple( - typename NC::Menu::ConstPropertiesIterator(it), - boost::make_transform_iterator(it, std::forward(map)) - ))); -} - -#endif // NCMPCPP_HELPERS_SONG_ITERATOR_MAKER_H +#endif // NCMPCPP_UTILITY_CONST_H -- 2.11.4.GIT